Move some folding for add/sub to gtFoldExpr#128200
Conversation
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Pull request overview
Continuation of #128187 — moves several morph-time simplifications for ADD/SUB/NEG/NOT (double-negation removal, (-a)+b => b-a, a+(-b) => a-b, a-(-b) => a+b, (-a)-(-b) => b-a) out of fgMorphSmpOp/fgOptimizeAddition into a refactored gtFoldExpr. The dispatch in gtFoldExpr is split into new gtFoldExprUnary / gtFoldExprBinary helpers, and a new GenTree::ToggleReverseOp helper is introduced so swaps can preserve evaluation order without consulting gtCanSwapOrder.
Changes:
- Refactor
gtFoldExprinto per-shape (Unary/Binary/Special) dispatch and add the corresponding folds forGT_NEG/GT_NOTandGT_ADD/GT_SUBwith NEG operands. - Remove the corresponding folds from
fgMorphSmpOpandfgOptimizeAddition. - Add
GenTree::ToggleReverseOp()to flipGTF_REVERSE_OPSafter swapping children.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/coreclr/jit/gentree.cpp | Refactors gtFoldExpr and adds new gtFoldExprUnary/gtFoldExprBinary with NEG/NOT/ADD/SUB folds; contains compile errors due to misspelled local references (op1Op1/op2Op1 vs op1op1/op2op1). |
| src/coreclr/jit/gentree.h | Adds ToggleReverseOp() helper to flip GTF_REVERSE_OPS. |
| src/coreclr/jit/compiler.h | Declares the new gtFoldExprUnary/gtFoldExprBinary helpers. |
| src/coreclr/jit/morph.cpp | Removes the simplifications now handled by gtFoldExprBinary/gtFoldExprUnary. |
b514a42 to
6b55ea0
Compare
6b55ea0 to
1f91448
Compare
fcc15a7 to
302fada
Compare
302fada to
9620f9a
Compare
9620f9a to
805470d
Compare
d5f71bc to
86e5292
Compare
87752bb to
ab47b7d
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/coreclr/jit/morph.cpp:7867
- Removing fgPushConstantsRight from these compare cases means the subsequent const-specific morph optimizations now only run when the constant is already in op2. For patterns that commonly arrive as CONST relop x (e.g., source-level
0 == x), this can skip fgOptimizeEqualityComparisonWithConst/fgOptimizeRelationalComparisonWithConst in Tier0/minopts (gtFoldExprSpecial won’t run there to normalize). Consider pushing constants right here again (or teaching the const optimizers to handle a constant in either operand) so these optimizations don’t depend on prior canonicalization.
case GT_EQ:
case GT_NE:
{
oper = tree->OperGet();
op1 = tree->gtGetOp1();
op2 = tree->gtGetOp2();
if (op2->IsIntegralConst())
{
tree = fgOptimizeEqualityComparisonWithConst(tree->AsOp());
| GenTree* Compiler::fgOptimizeCommutativeArithmetic(GenTreeOp* tree) | ||
| { | ||
| assert(tree->OperIs(GT_ADD, GT_MUL, GT_OR, GT_XOR, GT_AND)); | ||
| assert(!tree->gtOverflowEx()); | ||
|
|
||
| fgPushConstantsRight(tree); | ||
|
|
||
| if (fgOperIsBitwiseRotationRoot(tree->OperGet())) | ||
| { | ||
| GenTree* rotationTree = fgRecognizeAndMorphBitwiseRotation(tree); |
| assert(tree->OperIsBinary()); | ||
| assert(opts.OptimizationEnabled()); | ||
|
|
||
| assert(tree->OperKind() & GTK_BINOP); | ||
| if (tree->OperIsCommutative() || tree->OperIsCompare()) | ||
| { | ||
| fgPushConstantsRight(tree); | ||
| } | ||
|
|
||
| /* Filter out operators that cannot be folded here */ | ||
| if (oper == GT_CAST) | ||
| if (opts.OptimizationDisabled()) | ||
| { | ||
| return tree; | ||
| } |
Continuation of #128187, but this time covering some simplifications for add/sub operations.