[PATCH] D91415: [InstCombine] add multi-use demanded bits fold for add with low-bit mask

Simon Pilgrim via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Nov 14 09:18:28 PST 2020


RKSimon accepted this revision.
RKSimon added a comment.
This revision is now accepted and ready to land.

LGTM with a few minor comments



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:838
+      // just return the add's variable operand.
+      if ((*C & LowMask).isNullValue())
+        return I->getOperand(0);
----------------
spatel wrote:
> RKSimon wrote:
> > We could technically support non-uniform vectors here by matching m_Constant and using a similar ConstantExpr fold.
> Ok - I didn't see any signs that we had tried non-uniform in this file yet. Do you think we should do that in this patch or as a TODO/follow-up?
I'm happy for a TODO for now - but ideally we'd start getting as many combines supporting non-uniform constants (inc undef elements) as soon as possible.

Unfortunately I don't think llvm::MaskedValueIsZero will work for undef elements.


================
Comment at: llvm/test/Transforms/InstCombine/and.ll:1052
 
+; Multi-use demanded bits - 'add' doesn't change 'and'
+
----------------
spatel wrote:
> RKSimon wrote:
> > Vector tests?
> Basic splat is the last (positive) test.
cheers - if you could add the non-uniform test coverage now, even if support isn't added yet that'd be great.


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

https://reviews.llvm.org/D91415



More information about the llvm-commits mailing list