Make bounds check nodes ordinary binary opers #59963
Conversation
ghost
added
the
community-contribution
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT Issue DetailsThere is no real reason why they cannot be, and making them enables the deletion of some custom traversal code:
It also enables The aforementioned diffs: pending.
|
SingleAccretion
force-pushed
the
Make-Bounds-Checks-Simple-BinOps
branch
3 times, most recently
from
3bebae9 to
3d3f36b
Compare
|
@dotnet/jit-contrib |
SingleAccretion
force-pushed
the
Make-Bounds-Checks-Simple-BinOps
branch
from
3d3f36b to
88cddac
Compare
There is no real reason why they cannot be, and making them enables the deletion of some custom traversal code. It also enables GTF_REVERSE_OPS, though the diffs from that appear to be mixed at best. But it did not seem worth it to quirk anything, as they are also small.
SingleAccretion
force-pushed
the
Make-Bounds-Checks-Simple-BinOps
branch
from
88cddac to
8e81de0
Compare
|
How hard would it be to get this to a zero-diff change? Would it just involve blocking I'd rather not see a refactoring change that also alters behavior. |
To get to zero diffs. Additionally, the less GTF_REVERSE_OPS we have, the better.
They are no longer needed now that GTF_REVERSE_OPS has been disabled. Note that these changes were CQ-only, i. e. in the event that some odd code sets GTF_REVERSE_OPS on the bounds checks nodes, everything will still work correctly.
To get to zero diffs.
Agree, bad judgement on my part. The latest commits contain changes making this strictly zero-diff across all of SPMI. |
There is no real reason why they cannot be, and making them binary enables the deletion of some custom traversal code:
+179 −508Outdated
It also enables
GTF_REVERSE_OPS, though the diffs from that appear to be mixed at best (they're all due to different register allocation, as one would expect). But it did not seem worth it to quirk anything, as they are also small. I've spent quite a bit of time checking various regressions for obvious bad costing (causing said regressions), but did not find anything notable.The aforementioned diffs:
win-x64,win-arm64,win-x86,linux-arm(the most affected target due to the flakiness of 16 vs 32 bit encodings).The bounds check node has also been made small, because there's nothing preventing it from being small. This saves a little bit of memory:
-0.033%when prejitting CoreLib.