[PATCH] D63812: [InstCombine] Shift amount reassociation (PR42391)
Roman Lebedev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sat Jun 29 04:30:45 PDT 2019
lebedev.ri marked 2 inline comments as done.
lebedev.ri added a comment.
Thanks for the review!
Other than D63829 <https://reviews.llvm.org/D63829>, unless that is magically sufficient to fix all the
problems i'm observing, i suspect i will have some more folds like this...
================
Comment at: lib/Transforms/InstCombine/InstCombineShifts.cpp:37
+ return nullptr;
+ auto *Sh1 = cast<BinaryOperator>(Sh1Value);
+
----------------
nikic wrote:
> Personally I'd prefer `auto *Sh1 = cast<BinaryOperator>(Sh0->getOperand(0))` over the use of `m_CombineAnd()` here, which makes the match less straightforward.
It really differs on case-by-case basis. I think `m_CombineAnd()` may be better in general,
at least compared to backend lack of it.
================
Comment at: lib/Transforms/InstCombine/InstCombineShifts.cpp:41
+ Instruction::BinaryOps ShiftOpcode = Sh0->getOpcode();
+ if (ShiftOpcode != cast<Instruction>(Sh0->getOperand(0))->getOpcode())
+ return nullptr;
----------------
nikic wrote:
> Can replace `cast<Instruction>(Sh0->getOperand(0))` with `Sh1` here.
Whoops, yes, i wanted to do that but forgot. Thank you.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63812/new/
https://reviews.llvm.org/D63812
More information about the llvm-commits
mailing list