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

[Thomas Raoux via Phabricator via llvm-commits]

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


================
Comment at: llvm/include/llvm/IR/Constants.h:771
+  // Cache whether or not the constant is a splat.
+  mutable Optional<bool> IsSplat;
+  bool isSplatData() const;
----------------
majnemer wrote:
> You could save a byte by rolling your own optional. I think this is valuable because there may be many `ConstantDataVector`.
> 
> Maybe something like:
>   bool IsSplatSet : 1;
>   bool IsSplat : 1;
Done.


================
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:
> ThomasRaoux wrote:
> > 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? 
> I was just thinking of floating this bit up:
>   if (DemandedElts.isNullValue())
>     return true;
What I meant is that DemandedLHS DemandedRHS are out parameters and do need to be set before return true. So we cannot move this above the APInt above.


================
Comment at: llvm/lib/IR/ConstantFold.cpp:1389
+      return UndefValue::get(VTy);
+    if (C1Splat && C2Splat) {
+      return ConstantVector::getSplat(
----------------
majnemer wrote:
> Now that these are lazy, maybe only do `C1->getSplatValue()` if C2Splat is true.
Done.


================
Comment at: llvm/lib/IR/Constants.cpp:2907
+bool ConstantDataVector::isSplat() const {
+  if(!IsSplat.hasValue())
+    IsSplat = isSplatData();
----------------
majnemer wrote:
> Formatting.
Done.


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

https://reviews.llvm.org/D76664