[PATCH] D131492: [RISCV] Implement isMaskAndCmp0FoldingBeneficial hook

[Alex Bradbury via Phabricator via llvm-commits]

asb added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1147
+  // extension is supported and the mask is a power of two. However, we
+  // conservatively return false if the mask would bit in an ANDI instruction,
+  // on the basis that it's possible the fold wouldn't decrease the
----------------
craig.topper wrote:
> craig.topper wrote:
> > reames wrote:
> > > This bit of logic feels like you're covering up something else.  Moving around the and instruction in IR shouldn't directly cause the bexti to be used if the andi form is legal and profitable.  Do we maybe have a missing pattern?
> > > 
> > > As a guess, maybe this is intersecting with D131482?
> > This hook enables copying of the AND not just moving. If we're just going to emit an ANDI, it doesn't really make sense to enable copying it.
> 'would bit' -> 'would fit'?
This hook is only used from CodeGenPrepare, to decide whether to sink an AND or not. I've updated the comment to hopefully better reflect the reasoning here.

Based on the only current in-tree use, a more descriptive name for this hook might be "shouldSinkAndToEnableMaskAndCmp0Folding".


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

https://reviews.llvm.org/D131492