[PATCH] D76664: [ConstantFold][NFC] Compile time optimization for large vectors

[Thomas Raoux via Phabricator via llvm-commits]

ThomasRaoux marked 2 inline comments as done.
ThomasRaoux added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:179
+  if (isa<ConstantAggregateZero>(Shuf->getMask())) {
+    DemandedLHS.setBit(0);
+    return true;
----------------
majnemer wrote:
> ThomasRaoux wrote:
> > majnemer wrote:
> > > Is it OK to set DemandedLHS bit 0 when DemandedElts[0] is false?
> > It is okay as long as DemandedElts has at least one bit set. With current code this function would never be called with an empty DemandedElts but I added an early check for this case to handle this case.
> Is it OK that the loop bellow only sets DemandedLHS[0] if DemandedElts[i] is true for some multiple of NumElts while this logic will set it unconditionally?
DemandedLHS[0] is set if for any DemandedElts[i] Shuf->getMaskValue(i) is 0 or NumElts. 
In this case we know all the getMaskValue(i) are equal to 0 (since the shuffle mask is aggregateZero) so if any DemandedElts[i] is set it's associated Shuf->getMaskValue(i) will be 0 and DemandedLHS[0] will be set. Since we handle the case where all the bits of DemandedElts are 0 above we know at this point that at least one bit is set.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:177-178
   DemandedLHS = DemandedRHS = APInt::getNullValue(NumElts);
-
+  if (DemandedElts.isNullValue())
+    return true;
+  // Simple case of a shuffle with zeroinitializer.
----------------
majnemer wrote:
> I'd float this to the top before we create more APInts.
I believe we still need to set DemandedLHS and DemandedRHS to APInt::getNullValue(NumElts) so I cannot really move it up? 


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

https://reviews.llvm.org/D76664