[PATCH] D87464: [TargetLowering] Improve SimplifyDemandedBits for AND and OR

Mon Sep 14 07:49:51 PDT 2020

RKSimon added inline comments.

================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:1271
+    // known bits in Op0, try simplifying Op1 again.
+    APInt Op1DemandedBits = ~Known2.One & DemandedBits;
+    if (Op1DemandedBits != DemandedBits &&
----------------
foad wrote:
> foad wrote:
> > craig.topper wrote:
> > > Can we trust Known2.One's value is correct for the bits that weren't checked due to Known.One being used to filter the DemandedBits for the previous call?
> > No we can't according to the comment on SimplifyDemandedBits:
> > ```
> >   /// expression (used to simplify the caller).  The KnownZero/One bits may only
> >   /// be accurate for those bits in the Demanded masks.
> > ```
> > This was a pre-existing problem with the call on line 1264.
> It seems to me that there are various pre-existing problems in this area.
> 
> This should be a no-op but causes test case diffs: https://github.com/jayfoad/llvm-project/commit/dfdc519d44ac4f5ad00d9d5322d641e2fcc5881a
> 
> This fixes the pre-existing problem I pointed out on line 1264, but causes various code quality regressions: https://github.com/jayfoad/llvm-project/commit/90a5c28ba29edf54fb22ea36bf1ba2625ce557a4
> 
> I'm not sure how to proceed now. Should we try to get to a state where known bits information **is** trustworthy for undemanded bits?
If you're willing to try to do the audit then by all means - we should at least be able to get to a position where all the KnownBits One/Zero bits will not be set unless they are definitely known - but some cases (e.g. bitcasts through vectors, partial demanded elts etc.) we might need to play safe.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87464