[PATCH] D68930: [InstCombine] Shift amount reassociation in shifty sign bit test (PR43595)

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 18 10:11:50 PDT 2019


lebedev.ri added a comment.

In D68930#1714734 <https://reviews.llvm.org/D68930#1714734>, @spatel wrote:

> In D68930#1713151 <https://reviews.llvm.org/D68930#1713151>, @lebedev.ri wrote:
>
> > Any thoughts on this?
>
>
> It raises the usual questions: cost/complexity vs. benefit, and how much further do we want to take this pattern matching before moving it somewhere else (AggressiveInstCombine)?
>  I don't think we'll reach consensus on that in this patch


Indeed. I was hoping that it wouldn't come to this patch, i thought general folds
(`reassociateShiftAmtsOfTwoSameDirectionShifts()`, `foldShiftIntoShiftInAnotherHandOfAndInICmp()`)
would handle it, but no, sadly i'm **still** seeing regressions (although smaller!) in target benchmark,
and analysis of IR points to *this* missing pattern. I can't yet tell **what** //other// folds are needed though.

> so I've just noted a small improvement.

Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68930/new/

https://reviews.llvm.org/D68930





More information about the llvm-commits mailing list