JIT: Unify post-order cmp transforms intofgOptimizeCmp#127883
JIT: Unify post-order cmp transforms intofgOptimizeCmp#127883BoyBaykiller wants to merge 5 commits intodotnet:mainfrom
fgOptimizeCmp#127883Conversation
github-actions
Bot
added
the
area-CodeGen-coreclr
dotnet-policy-service
Bot
added
the
community-contribution
| return cmp; | ||
| } | ||
|
|
||
| // TODO-CQ: This should be called for all comparisons |
There was a problem hiding this comment.
This preserves previous behaviour to keep 0 diffs. But in a follow up PR I will enable this for EQ/NE too
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
|
||
| //------------------------------------------------------------------------ | ||
| // fgOptimizeRelationalComparisonWithFullRangeConst: optimizes a comparison operation. | ||
| // fgOptimizeCmpLtLeGeGtFullRangeConst: optimizes a comparison operation. |
There was a problem hiding this comment.
this name is odd, the previous was better IMO. in other places we use the word "relops"
There was a problem hiding this comment.
I think it should accept all relops including EQ and NE, and then just call the other func for EQ/NE
There was a problem hiding this comment.
I think it should accept all relops including EQ and NE, and then just call the other func for EQ/NE
As far as I understand the two corresponding functions are
fgOptimizeCmpEqNeWithConst (for Eq/Ne)
fgOptimizeCmpLtLeGeGtWithConst (for the others)
but fgOptimizeCmpLtLeGeGtFullRangeConst is it's own thing which can return a cns tree while the other twp only modify the oper. So I don't think I should start calling fgOptimizeCmpEqNeWithConst in here.
However I will follow tanners advice and remove the LtLeGeGt part here.
this name is odd, the previous was better IMO. in other places we use the word "relops"
RelationalComparison is kinda long. I liked "cmp" because the optimizations apply to all tree->OperIsCmpCompare() trees so the names match. However I know we also use "relop" a bunch so I'd be fine with that too.
|
@EgorBo does this need any other changes? If I understood correctly "cmp" over "relop" is ok. |
2 participants
Zero diffs.