[PATCH] D152568: [InstCombine] Transform `(binop1 (binop2 (lshift X,Amt),Mask),(lshift Y,Amt))`
Nikita Popov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun Jun 11 09:30:43 PDT 2023
nikic added a comment.
It looks like there are some more tests that need to be updated.
================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:771
+ // LHS and RHS need same shift opcode
+ ShOpc = IY->getOpcode();
+ if (ShOpc != IX->getOpcode())
----------------
Here and elsewhere: Keep variable scope minimal.
================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:775
+
+ // Make sure binop is real instruction and not ContstantExpr
+ auto *BO2 = dyn_cast<Instruction>(I.getOperand(1 - ShOpnum));
----------------
ConstantExpr
================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:785
+
+ // If BinOp1 == BinOp2 and its bitwise or shl with add, then just
+ // reassosiate to drop the shift irrelivant of constants.
----------------
it's
================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:786
+ // If BinOp1 == BinOp2 and its bitwise or shl with add, then just
+ // reassosiate to drop the shift irrelivant of constants.
+ if (BinOpc == I.getOpcode() &&
----------------
irrelevant
================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:795
+
+ // Otherwise we can only reassosiate by constant shifting the mask, so
+ // ensure we have constants.
----------------
reassociate
Though this is really more "distribute" than reassociate.
================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:812
+ // Pass
+ }
+ // If both BinOp1 and BinOp2 are bitwise ops and:
----------------
Does this case trigger for anything other than non-splat vectors? If not, please remove it. (Generally, don't add non-splat vector support it it requires //any// additional complexity.)
================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:820
+ ConstantExpr::get(InvShOpc, CMask, CShift),
+ CShift) == CMask) {
+ // Pass
----------------
Isn't this also fine for add/shl, as in the case above? https://alive2.llvm.org/ce/z/ztthn5
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152568/new/
https://reviews.llvm.org/D152568
More information about the llvm-commits
mailing list