Skip to content

Add RTCHECK node for compiler-inserted runtime check code#114369

Closed
snickolls-arm wants to merge 1 commit intodotnet:mainfrom
snickolls-arm:runtime-check-node
Closed

Add RTCHECK node for compiler-inserted runtime check code#114369
snickolls-arm wants to merge 1 commit intodotnet:mainfrom
snickolls-arm:runtime-check-node

Conversation

@snickolls-arm
Copy link
Contributor

Introduces a new node type the compiler can introduce to create a runtime check that happens before a value is evaluated in the code tree.

The node wraps some 'value' tree, and also depends on a 'check' tree. The check tree contains code that performs the runtime check, and the value tree is the actual value of this tree during normal execution (the runtime check does not pass).

The node is marked such that it may throw an exception. Codegen should generate code for the check tree, and jump to throw an exception if the check tree evaluates to true. Otherwise, the node will be assigned a register value representing its wrapped value which can be used/forwarded as necessary.

This patch implements this node for divide-by-zero and overflow checks on ARM64.

Introduces a new node type the compiler can introduce to create a
runtime check that happens before a value is evaluated in the code tree.

The node wraps some 'value' tree, and also depends on a 'check' tree.
The check tree contains code that performs the runtime check, and the
value tree is the actual value of this tree during normal execution
(the runtime check does not pass).

The node is marked such that it may throw an exception. Codegen should
generate code for the check tree, and jump to throw an exception if the
check tree evaluates to true. Otherwise, the node will be assigned a
register value representing its wrapped value which can be
used/forwarded as necessary.

This patch implements this node for divide-by-zero and overflow checks
on ARM64.
@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Apr 8, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Apr 8, 2025
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

@snickolls-arm
Copy link
Contributor Author

@jakobbotsch Follow on from #111543

This is an early draft of changes towards adding a new node to represent a runtime check. I've created a generalized node and then specialized it to divide-by-zero and division overflow.

The first thing I've noticed is that it isn't able to optimize the flow graph like the QMARK implementation. Probably because processing statically decided checks happens quite late. I added some code to process these in rationalization but it would need to happen before flow graph optimizations. I haven't looked at assertion propagation, maybe this would help?

//
struct GenTreeRTCheck : public GenTreeOp
{
SpecialCodeKind gtThrowKind; // Kind of throw block to branch to on failure
Copy link
Member

@jakobbotsch jakobbotsch Apr 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think using this pattern for the overflow check is going to come with any benefits? Optimizing multiple of those checks will generally require both the dividend and divisor to be equal, but in that case the whole division operation is also equal and can be CSE'd on its own.
I think the main interest is for the zero check, so I would just make it GT_ZEROCHECK and make it a normal GenTreeOp.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote it this way partly thinking about making it easier to add new checks in future. Having a common framework for if (expr) throw() could be helpful generally. I don't expect any performance benefit from doing it this way, except that it does allow the JIT to generate CMP+CCMP for the overflow check at the moment, which is good for lowering conditional branch density. I could just change the emitter to generate this pattern though, if it shows a benefit. I did something similar with the divide-by-zero checks recently (#111797).

GenTreeRTCheck* Compiler::gtNewDivideByZeroCheck(GenTree* divisor)
{
assert(varTypeIsIntegral(divisor));
GenTree* check = gtNewOperNode(GT_EQ, TYP_INT, gtCloneExpr(divisor), gtNewIconNode(0, genActualType(divisor)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not legal to clone the divisor tree like this since it can have side effects. I would just remove this and call gtNewRTCheckNode directly from the importer, where we can ensure the proper side effect handling.

@dotnet-policy-service
Copy link
Contributor

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@github-actions github-actions bot locked and limited conversation to collaborators Jun 8, 2025
@snickolls-arm snickolls-arm deleted the runtime-check-node branch August 7, 2025 12:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants