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

Thomas Raoux via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 23 23:22:16 PDT 2020


ThomasRaoux marked 8 inline comments as done.
ThomasRaoux added a comment.

Thanks David.



================
Comment at: llvm/include/llvm/IR/Constants.h:770
+      : ConstantDataSequential(ty, ConstantDataVectorVal, Data) {
+    IsSplat = isSplatData(ty, Data);
+  }
----------------
majnemer wrote:
> I'd move this into the initializer.
Done.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:179
+  if (isa<ConstantAggregateZero>(Shuf->getMask())) {
+    DemandedLHS.setBit(0);
+    return true;
----------------
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.


================
Comment at: llvm/lib/IR/ConstantFold.cpp:64-67
+  if (Constant *Splat = CV->getSplatValue()) {
+    return ConstantVector::getSplat(DstTy->getVectorElementCount(),
+                                    ConstantExpr::getBitCast(Splat, DstEltTy));
+  }
----------------
majnemer wrote:
> Please add a comment for this.
Done.


================
Comment at: llvm/lib/IR/ConstantFold.cpp:891-896
+  if (isa<ConstantAggregateZero>(Mask) && !MaskEltCount.Scalable) {
+    Type *Ty = IntegerType::get(V1->getContext(), 32);
+    Constant *Elt =
+        ConstantExpr::getExtractElement(V1, ConstantInt::get(Ty, 0));
+    return ConstantVector::getSplat(MaskEltCount, Elt);
+  }
----------------
majnemer wrote:
> Please add a comment here.
Done.


================
Comment at: llvm/lib/IR/ConstantFold.cpp:1385-1386
+    if (C1Splat && C2Splat) {
+      if (Instruction::isIntDivRem(Opcode) && C2Splat->isNullValue())
+        return UndefValue::get(VTy);
+      return ConstantVector::getSplat(
----------------
majnemer wrote:
> This doesn't depend on `C1`.
Right, I pulled this part out of the if.


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

https://reviews.llvm.org/D76664





More information about the llvm-commits mailing list