[SVE]Clarify TypeSize comparisons in llvm/lib/Transforms

[CarolineConcatto]

Summary

Use isKnownXY comparators when one of the operands can be with
scalable vectors or getFixedSize() for all the other cases.

This patch also does bug fixes for getPrimitiveSizeInBits by using
getFixedSize() near the places with the TypeSize comparison.

[david-arm]

llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp

2023–2024

I think it's safe to use getFixedSize() here instead of TypeSize::isKnownLT because we know from the checks just above that these types must be integer types.

[sdesmalen]

llvm/lib/Transforms/IPO/GlobalOpt.cpp

1883–1884

Globals are not allowed to be scalable, so this shouldn't use isKnownLE.

llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp

899–900

Does this need to work for scalable types?

llvm/lib/Transforms/Scalar/NaryReassociate.cpp

378–379

This is a functional change that now allows mixing scalable and fixed-width types. Either that should not happen (and thus it shouldn't use isKnownLT) or it should have a test that tests the new behaviour.

[CarolineConcatto]

-address review's comments

[CarolineConcatto]

Thank you Sander and David.
I've changed the points you've raised.
I had to run some test to make sure no scalable vectors would reach the places you've raised.
We can relax, atm no scalable vectors can reach the paths where we are using getFixedSize().

llvm/lib/Transforms/IPO/GlobalOpt.cpp

1883–1884

Thank you Sander.

llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp

899–900

After running some tests I can confirm this path will never be accessible by a scalable vector.
The test inside Transforms/LoopIdiom/memcpy-vectors.ll confirms that this patch will never be reached.

llvm/lib/Transforms/Scalar/NaryReassociate.cpp

378–379

No vector(fix-width or scalable) will go down this path
The test:
if (SE->isSCEVable(I->getType()) && isPotentiallyNaryReassociable(&*I))
inside the method:
bool NaryReassociatePass::doOneIteration(Function &F)
prevents these types to reach this path.

llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp

2023–2024

Good catch @david-arm.
Thank you for spending the time on it.

[sdesmalen]

LGTM!

llvm/lib/Transforms/Scalar/NaryReassociate.cpp

378–379

Okay that makes sense, isSCEVable returns false on vector types. Thanks for confirming!

llvm/lib/Transforms/IPO/GlobalOpt.cpp

static bool isPointerValueDeadOnEntryToFunction(
   for (auto *L : Loads) {
     auto *LTy = L->getType();
     if (none_of(Stores, [&](const StoreInst *S) {
           auto *STy = S->getValueOperand()->getType();
           // The load is only dominated by the store if DomTree says so
           // and the number of bits loaded in L is less than or equal to
           // the number of bits stored in S.
           return DT.dominates(S, L) &&
-                 DL.getTypeStoreSize(LTy) <= DL.getTypeStoreSize(STy);
+                 DL.getTypeStoreSize(LTy).getFixedSize() <=
+                     DL.getTypeStoreSize(STy).getFixedSize();
         }))
       return false;
   }
   // All loads have known dependences inside F, so the global can be localized.
   return true;
 }
 
 /// C may have non-instruction users. Can all of those users be turned into

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

InstCombinerImpl::foldCmpLoadFromIndexedGlobal(GetElementPtrInst *GEP,
   Value *Idx = GEP->getOperand(2);
 
   // If the index is larger than the pointer size of the target, truncate the
   // index down like the GEP would do implicitly.  We don't have to do this for
   // an inbounds GEP because the index can't be out of range.
   if (!GEP->isInBounds()) {
     Type *IntPtrTy = DL.getIntPtrType(GEP->getType());
     unsigned PtrSize = IntPtrTy->getIntegerBitWidth();
-    if (Idx->getType()->getPrimitiveSizeInBits() > PtrSize)
+    if (Idx->getType()->getPrimitiveSizeInBits().getFixedSize() > PtrSize)
       Idx = Builder.CreateTrunc(Idx, IntPtrTy);
   }
 
   // If the comparison is only true for one or two elements, emit direct
   // comparisons.
   if (SecondTrueElement != Overdefined) {
     // None true -> false.
     if (FirstTrueElement == Undefined)
static Value *evaluateGEPOffsetExpression(User *GEP, InstCombinerImpl &IC,
   // pointer/array/vector index.  If there is no offset, life is simple, return
   // the index.
   Type *IntPtrTy = DL.getIntPtrType(GEP->getOperand(0)->getType());
   unsigned IntPtrWidth = IntPtrTy->getIntegerBitWidth();
   if (Offset == 0) {
     // Cast to intptrty in case a truncation occurs.  If an extension is needed,
     // we don't need to bother extending: the extension won't affect where the
     // computation crosses zero.
-    if (VariableIdx->getType()->getPrimitiveSizeInBits() > IntPtrWidth) {
+    if (VariableIdx->getType()->getPrimitiveSizeInBits().getFixedSize() >
+        IntPtrWidth) {
       VariableIdx = IC.Builder.CreateTrunc(VariableIdx, IntPtrTy);
     }
     return VariableIdx;
   }
 
   // Otherwise, there is an index.  The computation we will do will be modulo
   // the pointer size.
   Offset = SignExtend64(Offset, IntPtrWidth);
if (PtrBase != GEPRHS->getOperand(0)) {
         Value *ROffset = EmitGEPOffset(GEPRHS);
 
         // If we looked through an addrspacecast between different sized address
         // spaces, the LHS and RHS pointers are different sized
         // integers. Truncate to the smaller one.
         Type *LHSIndexTy = LOffset->getType();
         Type *RHSIndexTy = ROffset->getType();
         if (LHSIndexTy != RHSIndexTy) {
-          if (LHSIndexTy->getPrimitiveSizeInBits() <
-              RHSIndexTy->getPrimitiveSizeInBits()) {
+          if (LHSIndexTy->getPrimitiveSizeInBits().getFixedSize() <
+              RHSIndexTy->getPrimitiveSizeInBits().getFixedSize()) {
             ROffset = Builder.CreateTrunc(ROffset, LHSIndexTy);
           } else
             LOffset = Builder.CreateTrunc(LOffset, RHSIndexTy);
         }
 
         Value *Cmp = Builder.CreateICmp(ICmpInst::getSignedPredicate(Cond),
                                         LOffset, ROffset);
         return replaceInstUsesWith(I, Cmp);

llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp

static const SCEV *getNumBytes(const SCEV *BECount, Type *IntPtr,
                                const DataLayout *DL, ScalarEvolution *SE) {
   const SCEV *NumBytesS;
   // The # stored bytes is (BECount+1)*Size.  Expand the trip count out to
   // pointer size if it isn't already.
   //
   // If we're going to need to zero extend the BE count, check if we can add
   // one to it prior to zero extending without overflow. Provided this is safe,
   // it allows better simplification of the +1.
-  if (DL->getTypeSizeInBits(BECount->getType()) <
-          DL->getTypeSizeInBits(IntPtr) &&
+  if (DL->getTypeSizeInBits(BECount->getType()).getFixedSize() <
+          DL->getTypeSizeInBits(IntPtr).getFixedSize() &&
       SE->isLoopEntryGuardedByCond(
           CurLoop, ICmpInst::ICMP_NE, BECount,
           SE->getNegativeSCEV(SE->getOne(BECount->getType())))) {
     NumBytesS = SE->getZeroExtendExpr(
         SE->getAddExpr(BECount, SE->getOne(BECount->getType()), SCEV::FlagNUW),
         IntPtr);
   } else {
     NumBytesS = SE->getAddExpr(SE->getTruncateOrZeroExtend(BECount, IntPtr),

llvm/lib/Transforms/Scalar/LoopPredication.cpp

 // This function returns true if we can safely represent the IV type in
 // the RangeCheckType without loss of information.
 static bool isSafeToTruncateWideIVType(const DataLayout &DL,
                                        ScalarEvolution &SE,
                                        const LoopICmp LatchCheck,
                                        Type *RangeCheckType) {
   if (!EnableIVTruncation)
     return false;
-  assert(DL.getTypeSizeInBits(LatchCheck.IV->getType()) >
-             DL.getTypeSizeInBits(RangeCheckType) &&
+  assert(DL.getTypeSizeInBits(LatchCheck.IV->getType()).getFixedSize() >
+             DL.getTypeSizeInBits(RangeCheckType).getFixedSize() &&
          "Expected latch check IV type to be larger than range check operand "
          "type!");
   // The start and end values of the IV should be known. This is to guarantee
   // that truncating the wide type will not lose information.
   auto *Limit = dyn_cast<SCEVConstant>(LatchCheck.Limit);
   auto *Start = dyn_cast<SCEVConstant>(LatchCheck.IV->getStart());
   if (!Limit || !Start)
     return false;
   // This check makes sure that the IV does not change sign during loop
   // iterations. Consider latchType = i64, LatchStart = 5, Pred = ICMP_SGE,
   // LatchEnd = 2, rangeCheckType = i32. If it's not a monotonic predicate, the
   // IV wraps around, and the truncation of the IV would lose the range of
   // iterations between 2^32 and 2^64.
   bool Increasing;
   if (!SE.isMonotonicPredicate(LatchCheck.IV, LatchCheck.Pred, Increasing))
     return false;
   // The active bits should be less than the bits in the RangeCheckType. This
   // guarantees that truncating the latch check to RangeCheckType is a safe
   // operation.
-  auto RangeCheckTypeBitSize = DL.getTypeSizeInBits(RangeCheckType);
+  auto RangeCheckTypeBitSize =
+      DL.getTypeSizeInBits(RangeCheckType).getFixedSize();
   return Start->getAPInt().getActiveBits() < RangeCheckTypeBitSize &&
          Limit->getAPInt().getActiveBits() < RangeCheckTypeBitSize;
 }
 
 
 // Return an LoopICmp describing a latch check equivlent to LatchCheck but with
 // the requested type if safe to do so.  May involve the use of a new IV.
 static Optional<LoopICmp> generateLoopLatchCheck(const DataLayout &DL,
                                                  ScalarEvolution &SE,
                                                  const LoopICmp LatchCheck,
                                                  Type *RangeCheckType) {
 
   auto *LatchType = LatchCheck.IV->getType();
   if (RangeCheckType == LatchType)
     return LatchCheck;
   // For now, bail out if latch type is narrower than range type.
-  if (DL.getTypeSizeInBits(LatchType) < DL.getTypeSizeInBits(RangeCheckType))
+  if (DL.getTypeSizeInBits(LatchType).getFixedSize() <
+      DL.getTypeSizeInBits(RangeCheckType).getFixedSize())
     return None;
   if (!isSafeToTruncateWideIVType(DL, SE, LatchCheck, RangeCheckType))
     return None;
   // We can now safely identify the truncated version of the IV and limit for
   // RangeCheckType.
   LoopICmp NewLatchCheck;
   NewLatchCheck.Pred = LatchCheck.Pred;
   NewLatchCheck.IV = dyn_cast<SCEVAddRecExpr>(

llvm/lib/Transforms/Scalar/NaryReassociate.cpp

NaryReassociatePass::tryReassociateGEPAtIndex(GetElementPtrInst *GEP,
   // Look for GEP's closest dominator that has the same SCEV as GEP except that
   // the I-th index is replaced with LHS.
   SmallVector<const SCEV *, 4> IndexExprs;
   for (auto Index = GEP->idx_begin(); Index != GEP->idx_end(); ++Index)
     IndexExprs.push_back(SE->getSCEV(*Index));
   // Replace the I-th index with LHS.
   IndexExprs[I] = SE->getSCEV(LHS);
   if (isKnownNonNegative(LHS, *DL, 0, AC, GEP, DT) &&
-      DL->getTypeSizeInBits(LHS->getType()) <
-          DL->getTypeSizeInBits(GEP->getOperand(I)->getType())) {
+      DL->getTypeSizeInBits(LHS->getType()).getFixedSize() <
+          DL->getTypeSizeInBits(GEP->getOperand(I)->getType()).getFixedSize()) {
     // Zero-extend LHS if it is non-negative. InstCombine canonicalizes sext to
     // zext if the source operand is proved non-negative. We should do that
     // consistently so that CandidateExpr more likely appears before. See
     // @reassociate_gep_assume for an example of this canonicalization.
     IndexExprs[I] =
         SE->getZeroExtendExpr(IndexExprs[I], GEP->getOperand(I)->getType());
   }
   const SCEV *CandidateExpr = SE->getGEPExpr(cast<GEPOperator>(GEP),

llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp

SCEVExpander::replaceCongruentIVs(Loop *L, const DominatorTree *DT,
   for (PHINode &PN : L->getHeader()->phis())
     Phis.push_back(&PN);
 
   if (TTI)
     llvm::sort(Phis, [](Value *LHS, Value *RHS) {
       // Put pointers at the back and make sure pointer < pointer = false.
       if (!LHS->getType()->isIntegerTy() || !RHS->getType()->isIntegerTy())
         return RHS->getType()->isIntegerTy() && !LHS->getType()->isIntegerTy();
-      return RHS->getType()->getPrimitiveSizeInBits() <
-             LHS->getType()->getPrimitiveSizeInBits();
+      return RHS->getType()->getPrimitiveSizeInBits().getFixedSize() <
+             LHS->getType()->getPrimitiveSizeInBits().getFixedSize();
     });
 
   unsigned NumElim = 0;
   DenseMap<const SCEV *, PHINode *> ExprToIVMap;
   // Process phis from wide to narrow. Map wide phis to their truncation
   // so narrow phis can reuse them.
   for (PHINode *Phi : Phis) {
     auto SimplifyPHINode = [&](PHINode *PN) -> Value * {