Match-up type sizes for possible extensions, based on actual bit-size rather than rounded-up byte size.

[clin1]

Summary

Avoids asserting in SCEV when the target extension type is actually smaller than the source type.

[clin1]

Fuzzer fails are timeouts; very unlikely they are related to this diff.

[clin1]

Gentle ping... does anyone see any issues with this (hopefully) minor alteration?

[nikic]

LGTM

llvm/test/Transforms/LoopLoadElim/loop-i1-count.ll

46

Please adjust the test to avoid undef as much as possible -- it makes it more likely that the test will break in the future.

[fhahn]

llvm/test/Transforms/LoopLoadElim/loop-i1-count.ll

10

is there anything x86 specific about the test? If not please remove the target triple.

47

!llvm.loop needed?

[clin1]

Sharpened the test.

[clin1]

Updated --- thanks for your comments!

llvm/lib/Analysis/LoopAccessAnalysis.cpp

static bool isSafeDependenceDistance(const DataLayout &DL, ScalarEvolution &SE,
   // also guarantees that distance >= VF.
   //
   const uint64_t ByteStride = Stride * TypeByteSize;
   const SCEV *Step = SE.getConstant(BackedgeTakenCount.getType(), ByteStride);
   const SCEV *Product = SE.getMulExpr(&BackedgeTakenCount, Step);
 
   const SCEV *CastedDist = &Dist;
   const SCEV *CastedProduct = Product;
-  uint64_t DistTypeSize = DL.getTypeAllocSize(Dist.getType());
-  uint64_t ProductTypeSize = DL.getTypeAllocSize(Product->getType());
+  uint64_t DistTypeSizeBits = DL.getTypeSizeInBits(Dist.getType());
+  uint64_t ProductTypeSizeBits = DL.getTypeSizeInBits(Product->getType());
 
   // The dependence distance can be positive/negative, so we sign extend Dist;
   // The multiplication of the absolute stride in bytes and the
   // backedgeTakenCount is non-negative, so we zero extend Product.
-  if (DistTypeSize > ProductTypeSize)
+  if (DistTypeSizeBits > ProductTypeSizeBits)
     CastedProduct = SE.getZeroExtendExpr(Product, Dist.getType());
   else
     CastedDist = SE.getNoopOrSignExtend(&Dist, Product->getType());
 
   // Is  Dist - (BackedgeTakenCount * Step) > 0 ?
   // (If so, then we have proven (**) because |Dist| >= Dist)
   const SCEV *Minus = SE.getMinusSCEV(CastedDist, CastedProduct);
   if (SE.isKnownPositive(Minus))
void LoopAccessInfo::collectStridedAccess(Value *MemAccess) {
 
   const SCEV *StrideExpr = PSE->getSCEV(Stride);
   const SCEV *BETakenCount = PSE->getBackedgeTakenCount();
 
   // Match the types so we can compare the stride and the BETakenCount.
   // The Stride can be positive/negative, so we sign extend Stride;
   // The backedgeTakenCount is non-negative, so we zero extend BETakenCount.
   const DataLayout &DL = TheLoop->getHeader()->getModule()->getDataLayout();
-  uint64_t StrideTypeSize = DL.getTypeAllocSize(StrideExpr->getType());
-  uint64_t BETypeSize = DL.getTypeAllocSize(BETakenCount->getType());
+  uint64_t StrideTypeSizeBits = DL.getTypeSizeInBits(StrideExpr->getType());
+  uint64_t BETypeSizeBits = DL.getTypeSizeInBits(BETakenCount->getType());
   const SCEV *CastedStride = StrideExpr;
   const SCEV *CastedBECount = BETakenCount;
   ScalarEvolution *SE = PSE->getSE();
-  if (BETypeSize >= StrideTypeSize)
+  if (BETypeSizeBits >= StrideTypeSizeBits)
     CastedStride = SE->getNoopOrSignExtend(StrideExpr, BETakenCount->getType());
   else
     CastedBECount = SE->getZeroExtendExpr(BETakenCount, StrideExpr->getType());
   const SCEV *StrideMinusBETaken = SE->getMinusSCEV(CastedStride, CastedBECount);
   // Since TripCount == BackEdgeTakenCount + 1, checking:
   // "Stride >= TripCount" is equivalent to checking:
   // Stride - BETakenCount > 0
   if (SE->isKnownPositive(StrideMinusBETaken)) {

llvm/test/Transforms/LoopLoadElim/loop-i1-count.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -passes=loop-load-elim %s | FileCheck %s
+
+; The backedge taken count of this loop is an i1 type, and the IV is i8.
+; The math in LoopAccessAnalysis was rounding the type sizes to bytes and
+; believing them equal, causing a size mismatch.
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+
+@a = external dso_local local_unnamed_addr global [1 x i32], align 4
+
+define dso_local void @test(i8 %inc) local_unnamed_addr {
+; CHECK-LABEL: @test(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[FOR_COND3:%.*]]
+; CHECK:       for.cond3:
+; CHECK-NEXT:    [[H_0:%.*]] = phi i8 [ 0, [[ENTRY:%.*]] ], [ [[ADD:%.*]], [[COND_END_FOR_COND_CLEANUP_LOOPEXIT_CRIT_EDGE:%.*]] ]
+; CHECK-NEXT:    [[IDXPROM11:%.*]] = sext i8 [[H_0]] to i64
+; CHECK-NEXT:    [[ARRAYIDX27:%.*]] = getelementptr inbounds [1 x i32], [1 x i32]* @a, i64 0, i64 [[IDXPROM11]]
+; CHECK-NEXT:    br label [[FOR_BODY:%.*]]
+; CHECK:       cond.end.for.cond.cleanup.loopexit_crit_edge:
+; CHECK-NEXT:    [[ADD]] = add i8 [[H_0]], [[INC:%.*]]
+; CHECK-NEXT:    br label [[FOR_COND3]]
+; CHECK:       for.body:
+; CHECK-NEXT:    store i32 0, i32* [[ARRAYIDX27]], align 4
+; CHECK-NEXT:    br i1 true, label [[COND_END_FOR_COND_CLEANUP_LOOPEXIT_CRIT_EDGE]], label [[FOR_BODY]]
+;
+entry:
+  br label %for.cond3
+
+for.cond3:                                        ; preds = %cond.end.for.cond.cleanup.loopexit_crit_edge, %entry
+  %h.0 = phi i8 [ 0, %entry ], [ %add, %cond.end.for.cond.cleanup.loopexit_crit_edge ]
+  %idxprom11 = sext i8 %h.0 to i64
+  %arrayidx27 = getelementptr inbounds [1 x i32], [1 x i32]* @a, i64 0, i64 %idxprom11
+  br label %for.body
+
+cond.end.for.cond.cleanup.loopexit_crit_edge:     ; preds = %for.body
+  %add = add i8 %h.0, %inc
+  br label %for.cond3
+
+for.body:                                         ; preds = %for.body, %for.cond3
+  store i32 0, i32* %arrayidx27, align 4
+  br i1 true, label %cond.end.for.cond.cleanup.loopexit_crit_edge, label %for.body
+}