[PATCH] D77301: [TargetLowering] Improve expansion of FSHL/FSHR

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/test/CodeGen/X86/fshl.ll:123-124
+; X86-SLOW-NEXT:    shrl %eax
+; X86-SLOW-NEXT:    andb $31, %cl
+; X86-SLOW-NEXT:    xorb $31, %cl
+; X86-SLOW-NEXT:    shrl %cl, %eax
----------------
RKSimon wrote:
> RKSimon wrote:
> > foad wrote:
> > > foad wrote:
> > > > The `andb` is redundant here and in a bunch of other i32/i64 test cases. I would have thought the simplifyDemandedBits machinery could work that out. Any ideas why this isn't already working?
> > > Answering my own question, `X86InstrCompiler.td` has patterns that match `(shift x (and y, 31)) ==> (shift x, y)` but it has no chance of matching when there is an intervening `sub` or `xor` as in `(shift x (xor (and y, 31), 31))`.
> > It might be possible to extend X86DAGToDAGISel.isUnneededShiftMask to handle this - @craig.topper might be able to advise.
> @craig.topper What do you reckon? Could we handle the xor(and(x,bw-1),bw-1) pattern directly or even try to call SimplifyDemandedBits/SimplifyMultipleUseDemandedBits ?
Node creation inside of Select requires maintaining topological sorting so calling SimplifyDemandedBits/SimplifyMultipleUseDemandedBits is problematic.

Why isn't this combine from DAGCombiner.cpp visitXOR kicking in "fold (xor (and x, y), y) -> (and (not x), y) "? Is it because the (and x, y) has another use by the other shift until the isel pattern skips over it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77301