From 238bbcab441c283b3fb8781dc8c3827bf6c4d407 Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Thu, 14 Nov 2024 18:17:04 +0000 Subject: [PATCH 01/24] [DA] Dependence analysis does not handle array accesses of different sizes This fixes bug https://github.com/llvm/llvm-project/issues/16183 where the elements of a same array are accesses as i32 and i64. This is not handled by the array dependence analysis. --- llvm/lib/Analysis/DependenceAnalysis.cpp | 38 +++++++++++-------- .../DependenceAnalysis/DifferentAccessSize.ll | 22 +++++++++++ 2 files changed, 44 insertions(+), 16 deletions(-) create mode 100644 llvm/test/Analysis/DependenceAnalysis/DifferentAccessSize.ll diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp index dc0ed22dbcc0b..5aa6ef6b1a5c2 100644 --- a/llvm/lib/Analysis/DependenceAnalysis.cpp +++ b/llvm/lib/Analysis/DependenceAnalysis.cpp @@ -3596,14 +3596,10 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst) { return std::make_unique(Src, Dst); } - assert(isLoadOrStore(Src) && "instruction is not load or store"); - assert(isLoadOrStore(Dst) && "instruction is not load or store"); - Value *SrcPtr = getLoadStorePointerOperand(Src); - Value *DstPtr = getLoadStorePointerOperand(Dst); + const MemoryLocation &DstLoc = MemoryLocation::get(Dst); + const MemoryLocation &SrcLoc = MemoryLocation::get(Src); - switch (underlyingObjectsAlias(AA, F->getDataLayout(), - MemoryLocation::get(Dst), - MemoryLocation::get(Src))) { + switch (underlyingObjectsAlias(AA, F->getDataLayout(), DstLoc, SrcLoc)) { case AliasResult::MayAlias: case AliasResult::PartialAlias: // cannot analyse objects if we don't understand their aliasing. @@ -3617,16 +3613,15 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst) { break; // The underlying objects alias; test accesses for dependence. } - // establish loop nesting levels - establishNestingLevels(Src, Dst); - LLVM_DEBUG(dbgs() << " common nesting levels = " << CommonLevels << "\n"); - LLVM_DEBUG(dbgs() << " maximum nesting levels = " << MaxLevels << "\n"); - - FullDependence Result(Src, Dst, PossiblyLoopIndependent, CommonLevels); - ++TotalArrayPairs; + if (DstLoc.Size != SrcLoc.Size) { + // The dependence test gets confused if the size of the memory accesses + // differ. + LLVM_DEBUG(dbgs() << "can't analyze must alias with different sizes\n"); + return std::make_unique(Src, Dst); + } - unsigned Pairs = 1; - SmallVector Pair(Pairs); + Value *SrcPtr = getLoadStorePointerOperand(Src); + Value *DstPtr = getLoadStorePointerOperand(Dst); const SCEV *SrcSCEV = SE->getSCEV(SrcPtr); const SCEV *DstSCEV = SE->getSCEV(DstPtr); LLVM_DEBUG(dbgs() << " SrcSCEV = " << *SrcSCEV << "\n"); @@ -3641,6 +3636,17 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst) { LLVM_DEBUG(dbgs() << "can't analyze SCEV with different pointer base\n"); return std::make_unique(Src, Dst); } + + // establish loop nesting levels + establishNestingLevels(Src, Dst); + LLVM_DEBUG(dbgs() << " common nesting levels = " << CommonLevels << "\n"); + LLVM_DEBUG(dbgs() << " maximum nesting levels = " << MaxLevels << "\n"); + + FullDependence Result(Src, Dst, PossiblyLoopIndependent, CommonLevels); + ++TotalArrayPairs; + + unsigned Pairs = 1; + SmallVector Pair(Pairs); Pair[0].Src = SrcSCEV; Pair[0].Dst = DstSCEV; diff --git a/llvm/test/Analysis/DependenceAnalysis/DifferentAccessSize.ll b/llvm/test/Analysis/DependenceAnalysis/DifferentAccessSize.ll new file mode 100644 index 0000000000000..2dded8f3b13a1 --- /dev/null +++ b/llvm/test/Analysis/DependenceAnalysis/DifferentAccessSize.ll @@ -0,0 +1,22 @@ +; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 5 +; RUN: opt < %s -disable-output "-passes=print" -aa-pipeline=basic-aa 2>&1 \ +; RUN: | FileCheck %s + +; The dependence test does not handle array accesses of different sizes: i32 and i64. +; Bug 16183 - https://github.com/llvm/llvm-project/issues/16183 + +define i64 @bug16183_alias(ptr nocapture %A) { +; CHECK-LABEL: 'bug16183_alias' +; CHECK-NEXT: Src: store i32 2, ptr %arrayidx, align 4 --> Dst: store i32 2, ptr %arrayidx, align 4 +; CHECK-NEXT: da analyze - none! +; CHECK-NEXT: Src: store i32 2, ptr %arrayidx, align 4 --> Dst: %0 = load i64, ptr %A, align 8 +; CHECK-NEXT: da analyze - confused! +; CHECK-NEXT: Src: %0 = load i64, ptr %A, align 8 --> Dst: %0 = load i64, ptr %A, align 8 +; CHECK-NEXT: da analyze - none! +; +entry: + %arrayidx = getelementptr inbounds i32, ptr %A, i64 1 + store i32 2, ptr %arrayidx, align 4 + %0 = load i64, ptr %A, align 8 + ret i64 %0 +} From 164a0fa97cdd2fa5c3499221628afe595fc502e3 Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Fri, 17 Jan 2025 20:54:19 +0000 Subject: [PATCH 02/24] [DA] check memory offsets are multiples of elements size Check that the offsets from memory base address are multiples of the array element size for a pair of memory accesses in a dependence test. The patch adds a testcase that cannot be disambiguated by the DA. --- llvm/lib/Analysis/DependenceAnalysis.cpp | 98 ++++++++++++++++++- .../DependenceAnalysis/DifferentOffsets.ll | 25 +++++ .../DependenceAnalysis/MIVCheckConst.ll | 2 +- 3 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp index 5aa6ef6b1a5c2..cab10c704f665 100644 --- a/llvm/lib/Analysis/DependenceAnalysis.cpp +++ b/llvm/lib/Analysis/DependenceAnalysis.cpp @@ -3626,7 +3626,9 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst) { const SCEV *DstSCEV = SE->getSCEV(DstPtr); LLVM_DEBUG(dbgs() << " SrcSCEV = " << *SrcSCEV << "\n"); LLVM_DEBUG(dbgs() << " DstSCEV = " << *DstSCEV << "\n"); - if (SE->getPointerBase(SrcSCEV) != SE->getPointerBase(DstSCEV)) { + const SCEV *SrcBase = SE->getPointerBase(SrcSCEV); + const SCEV *DstBase = SE->getPointerBase(DstSCEV); + if (SrcBase != DstBase) { // If two pointers have different bases, trying to analyze indexes won't // work; we can't compare them to each other. This can happen, for example, // if one is produced by an LCSSA PHI node. @@ -3637,6 +3639,100 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst) { return std::make_unique(Src, Dst); } + auto EltSize = SrcLoc.Size.toRaw(); + assert(EltSize == DstLoc.Size.toRaw() && "Array element size differ"); + + // Check that memory access offsets in V are multiples of array EltSize. + std::function checkOffsets = + [&](const SCEV *V, const SCEV *&Param) -> bool { + if (auto *AddRec = dyn_cast(V)) { + if (!checkOffsets(AddRec->getStart(), Param)) + return false; + return checkOffsets(AddRec->getStepRecurrence(*SE), Param); + } + if (auto *Cst = dyn_cast(V)) { + APInt C = Cst->getAPInt(); + return C.srem(EltSize) == 0; + } + + auto checkParamsMultipleOfSize = [&](const SCEV *V, + const SCEV *&Param) -> bool { + if (EltSize == 1) + return true; + if (!Param) { + Param = V; + return true; + } + if (Param == V) + return true; + + // Check whether "(Param - V) % Size == 0". + const SCEV *Diff = SE->getMinusSCEV(Param, V); + if (const SCEVConstant *Cst = dyn_cast(Diff)) { + APInt Val = Cst->getAPInt(); + if (Val.isZero()) + return true; + auto Rem = + Val.srem(APInt(Val.getBitWidth(), EltSize, /*isSigned=*/true)); + if (Rem.isZero()) + return true; + LLVM_DEBUG(dbgs() << "SCEV with different offsets: " << *Param << " - " + << *V << " = " << *Diff << " % " << EltSize << " = " + << Rem << " != 0\n"); + return false; + } + // Check if the symbolic difference is a multiple of Size. + const SCEV *Val = SE->getConstant( + APInt(Diff->getType()->getScalarSizeInBits(), EltSize)); + + // Check by using the remainder computation. + const SCEV *Remainder = SE->getURemExpr(Diff, Val); + if (const SCEVConstant *C = dyn_cast(Remainder)) + if (C->getValue()->isZero()) + return true; + + // Check by using the division computation. + const SCEV *Q = SE->getUDivExpr(Diff, Val); + const SCEV *Product = SE->getMulExpr(Q, Val); + if (Diff == Product) + return true; + LLVM_DEBUG(dbgs() << "SCEV with different offsets:\n" + << *Param << " - " << *V << " = " << *Diff << "\n" + << "Remainder = " << *Remainder << "\n" + << "Q = " << *Q << " Product = " << *Product << "\n"); + return false; + }; + // Expressions like "n". + if (isa(V)) + return checkParamsMultipleOfSize(V, Param); + // Expressions like "n + 1". + if (isa(V)) + return !SCEVExprContains(V, [](const SCEV *S) { + return isa(S); + }) || checkParamsMultipleOfSize(V, Param); + + if (isa(V)) + return !SCEVExprContains(V, [](const SCEV *S) { + return isa(S); + }) || checkParamsMultipleOfSize(V, Param); + + LLVM_DEBUG(dbgs() << "SCEV node not handled yet: " << *V << "\n"); + return false; + }; + + // Check that memory access offsets are multiples of element sizes. + const SCEV *SrcEv = SE->getMinusSCEV(SrcSCEV, SrcBase); + const SCEV *DstEv = SE->getMinusSCEV(DstSCEV, DstBase); + const SCEV *Param = nullptr; + + if (Src != Dst) { + // Check that memory access offsets are multiples of element sizes. + if (!checkOffsets(SrcEv, Param) || !checkOffsets(DstEv, Param)) { + LLVM_DEBUG(dbgs() << "can't analyze SCEV with different offsets\n"); + return std::make_unique(Src, Dst); + } + } + // establish loop nesting levels establishNestingLevels(Src, Dst); LLVM_DEBUG(dbgs() << " common nesting levels = " << CommonLevels << "\n"); diff --git a/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll b/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll new file mode 100644 index 0000000000000..00375da1f26de --- /dev/null +++ b/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll @@ -0,0 +1,25 @@ +; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 5 +; RUN: opt < %s -disable-output "-passes=print" -aa-pipeline=basic-aa 2>&1 \ +; RUN: | FileCheck %s + +; The dependence test does not handle array accesses with difference between array accesses +; is not a multiple of the array element size. + +; In this test, the element size is i32 = 4 bytes and the difference between the +; load and the store is 2 bytes. + +define i32 @alias_with_different_offsets(ptr nocapture %A) { +; CHECK-LABEL: 'alias_with_different_offsets' +; CHECK-NEXT: Src: store i32 2, ptr %arrayidx, align 1 --> Dst: store i32 2, ptr %arrayidx, align 1 +; CHECK-NEXT: da analyze - none! +; CHECK-NEXT: Src: store i32 2, ptr %arrayidx, align 1 --> Dst: %0 = load i32, ptr %A, align 1 +; CHECK-NEXT: da analyze - confused! +; CHECK-NEXT: Src: %0 = load i32, ptr %A, align 1 --> Dst: %0 = load i32, ptr %A, align 1 +; CHECK-NEXT: da analyze - none! +; +entry: + %arrayidx = getelementptr inbounds i8, ptr %A, i64 2 + store i32 2, ptr %arrayidx, align 1 + %0 = load i32, ptr %A, align 1 + ret i32 %0 +} diff --git a/llvm/test/Analysis/DependenceAnalysis/MIVCheckConst.ll b/llvm/test/Analysis/DependenceAnalysis/MIVCheckConst.ll index fa58a81d2355b..c8fbbe60b0bf2 100644 --- a/llvm/test/Analysis/DependenceAnalysis/MIVCheckConst.ll +++ b/llvm/test/Analysis/DependenceAnalysis/MIVCheckConst.ll @@ -41,7 +41,7 @@ define void @test(ptr %A, ptr %B, i1 %arg, i32 %n, i32 %m) #0 align 2 { ; CHECK-NEXT: Src: %v27 = load <32 x i32>, ptr %v25, align 256 --> Dst: %v27 = load <32 x i32>, ptr %v25, align 256 ; CHECK-NEXT: da analyze - consistent input [0 S S]! ; CHECK-NEXT: Src: %v27 = load <32 x i32>, ptr %v25, align 256 --> Dst: %v32 = load <32 x i32>, ptr %v30, align 128 -; CHECK-NEXT: da analyze - input [* S S|<]! +; CHECK-NEXT: da analyze - confused! ; CHECK-NEXT: Src: %v32 = load <32 x i32>, ptr %v30, align 128 --> Dst: %v32 = load <32 x i32>, ptr %v30, align 128 ; CHECK-NEXT: da analyze - consistent input [0 S S]! ; From a479bdfa5b338563b4e4efb3d89b21dd28d6365b Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Wed, 26 Feb 2025 19:13:14 +0000 Subject: [PATCH 03/24] fix auto type --- llvm/lib/Analysis/DependenceAnalysis.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp index cab10c704f665..2ddc0c152c932 100644 --- a/llvm/lib/Analysis/DependenceAnalysis.cpp +++ b/llvm/lib/Analysis/DependenceAnalysis.cpp @@ -3639,7 +3639,7 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst) { return std::make_unique(Src, Dst); } - auto EltSize = SrcLoc.Size.toRaw(); + uint64_t EltSize = SrcLoc.Size.toRaw(); assert(EltSize == DstLoc.Size.toRaw() && "Array element size differ"); // Check that memory access offsets in V are multiples of array EltSize. From 9213c321181496a9c8387faf26030f8c503959df Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Wed, 26 Feb 2025 19:26:31 +0000 Subject: [PATCH 04/24] convert checkOffsets lambda to static function --- llvm/lib/Analysis/DependenceAnalysis.cpp | 158 +++++++++++------------ 1 file changed, 78 insertions(+), 80 deletions(-) diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp index 2ddc0c152c932..1ace975ddf9e3 100644 --- a/llvm/lib/Analysis/DependenceAnalysis.cpp +++ b/llvm/lib/Analysis/DependenceAnalysis.cpp @@ -3569,6 +3569,82 @@ bool DependenceInfo::invalidate(Function &F, const PreservedAnalyses &PA, Inv.invalidate(F, PA); } +static bool checkOffsets(ScalarEvolution *SE, const SCEV *V, const SCEV *&Param, + uint64_t EltSize) { + if (auto *AddRec = dyn_cast(V)) { + if (!checkOffsets(SE, AddRec->getStart(), Param, EltSize)) + return false; + return checkOffsets(SE, AddRec->getStepRecurrence(*SE), Param, EltSize); + } + if (auto *Cst = dyn_cast(V)) { + APInt C = Cst->getAPInt(); + return C.srem(EltSize) == 0; + } + + auto checkParamsMultipleOfSize = [&](const SCEV *V, + const SCEV *&Param) -> bool { + if (EltSize == 1) + return true; + if (!Param) { + Param = V; + return true; + } + if (Param == V) + return true; + + // Check whether "(Param - V) % Size == 0". + const SCEV *Diff = SE->getMinusSCEV(Param, V); + if (const SCEVConstant *Cst = dyn_cast(Diff)) { + APInt Val = Cst->getAPInt(); + if (Val.isZero()) + return true; + auto Rem = Val.srem(APInt(Val.getBitWidth(), EltSize, /*isSigned=*/true)); + if (Rem.isZero()) + return true; + LLVM_DEBUG(dbgs() << "SCEV with different offsets: " << *Param << " - " + << *V << " = " << *Diff << " % " << EltSize << " = " + << Rem << " != 0\n"); + return false; + } + // Check if the symbolic difference is a multiple of Size. + const SCEV *Val = + SE->getConstant(APInt(Diff->getType()->getScalarSizeInBits(), EltSize)); + + // Check by using the remainder computation. + const SCEV *Remainder = SE->getURemExpr(Diff, Val); + if (const SCEVConstant *C = dyn_cast(Remainder)) + if (C->getValue()->isZero()) + return true; + + // Check by using the division computation. + const SCEV *Q = SE->getUDivExpr(Diff, Val); + const SCEV *Product = SE->getMulExpr(Q, Val); + if (Diff == Product) + return true; + LLVM_DEBUG(dbgs() << "SCEV with different offsets:\n" + << *Param << " - " << *V << " = " << *Diff << "\n" + << "Remainder = " << *Remainder << "\n" + << "Q = " << *Q << " Product = " << *Product << "\n"); + return false; + }; + // Expressions like "n". + if (isa(V)) + return checkParamsMultipleOfSize(V, Param); + // Expressions like "n + 1". + if (isa(V)) + return !SCEVExprContains(V, [](const SCEV *S) { + return isa(S); + }) || checkParamsMultipleOfSize(V, Param); + + if (isa(V)) + return !SCEVExprContains(V, [](const SCEV *S) { + return isa(S); + }) || checkParamsMultipleOfSize(V, Param); + + LLVM_DEBUG(dbgs() << "SCEV node not handled yet: " << *V << "\n"); + return false; +} + // depends - // Returns NULL if there is no dependence. // Otherwise, return a Dependence with as many details as possible. @@ -3642,92 +3718,14 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst) { uint64_t EltSize = SrcLoc.Size.toRaw(); assert(EltSize == DstLoc.Size.toRaw() && "Array element size differ"); - // Check that memory access offsets in V are multiples of array EltSize. - std::function checkOffsets = - [&](const SCEV *V, const SCEV *&Param) -> bool { - if (auto *AddRec = dyn_cast(V)) { - if (!checkOffsets(AddRec->getStart(), Param)) - return false; - return checkOffsets(AddRec->getStepRecurrence(*SE), Param); - } - if (auto *Cst = dyn_cast(V)) { - APInt C = Cst->getAPInt(); - return C.srem(EltSize) == 0; - } - - auto checkParamsMultipleOfSize = [&](const SCEV *V, - const SCEV *&Param) -> bool { - if (EltSize == 1) - return true; - if (!Param) { - Param = V; - return true; - } - if (Param == V) - return true; - - // Check whether "(Param - V) % Size == 0". - const SCEV *Diff = SE->getMinusSCEV(Param, V); - if (const SCEVConstant *Cst = dyn_cast(Diff)) { - APInt Val = Cst->getAPInt(); - if (Val.isZero()) - return true; - auto Rem = - Val.srem(APInt(Val.getBitWidth(), EltSize, /*isSigned=*/true)); - if (Rem.isZero()) - return true; - LLVM_DEBUG(dbgs() << "SCEV with different offsets: " << *Param << " - " - << *V << " = " << *Diff << " % " << EltSize << " = " - << Rem << " != 0\n"); - return false; - } - // Check if the symbolic difference is a multiple of Size. - const SCEV *Val = SE->getConstant( - APInt(Diff->getType()->getScalarSizeInBits(), EltSize)); - - // Check by using the remainder computation. - const SCEV *Remainder = SE->getURemExpr(Diff, Val); - if (const SCEVConstant *C = dyn_cast(Remainder)) - if (C->getValue()->isZero()) - return true; - - // Check by using the division computation. - const SCEV *Q = SE->getUDivExpr(Diff, Val); - const SCEV *Product = SE->getMulExpr(Q, Val); - if (Diff == Product) - return true; - LLVM_DEBUG(dbgs() << "SCEV with different offsets:\n" - << *Param << " - " << *V << " = " << *Diff << "\n" - << "Remainder = " << *Remainder << "\n" - << "Q = " << *Q << " Product = " << *Product << "\n"); - return false; - }; - // Expressions like "n". - if (isa(V)) - return checkParamsMultipleOfSize(V, Param); - // Expressions like "n + 1". - if (isa(V)) - return !SCEVExprContains(V, [](const SCEV *S) { - return isa(S); - }) || checkParamsMultipleOfSize(V, Param); - - if (isa(V)) - return !SCEVExprContains(V, [](const SCEV *S) { - return isa(S); - }) || checkParamsMultipleOfSize(V, Param); - - LLVM_DEBUG(dbgs() << "SCEV node not handled yet: " << *V << "\n"); - return false; - }; - - // Check that memory access offsets are multiples of element sizes. const SCEV *SrcEv = SE->getMinusSCEV(SrcSCEV, SrcBase); const SCEV *DstEv = SE->getMinusSCEV(DstSCEV, DstBase); const SCEV *Param = nullptr; if (Src != Dst) { // Check that memory access offsets are multiples of element sizes. - if (!checkOffsets(SrcEv, Param) || !checkOffsets(DstEv, Param)) { + if (!checkOffsets(SE, SrcEv, Param, EltSize) || + !checkOffsets(SE, DstEv, Param, EltSize)) { LLVM_DEBUG(dbgs() << "can't analyze SCEV with different offsets\n"); return std::make_unique(Src, Dst); } From caf4f8d49211430c697825246e19b4770b0fc222 Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Wed, 26 Feb 2025 19:28:58 +0000 Subject: [PATCH 05/24] remove useless comment --- llvm/lib/Analysis/DependenceAnalysis.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp index 1ace975ddf9e3..4a730fdf45f86 100644 --- a/llvm/lib/Analysis/DependenceAnalysis.cpp +++ b/llvm/lib/Analysis/DependenceAnalysis.cpp @@ -3731,7 +3731,6 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst) { } } - // establish loop nesting levels establishNestingLevels(Src, Dst); LLVM_DEBUG(dbgs() << " common nesting levels = " << CommonLevels << "\n"); LLVM_DEBUG(dbgs() << " maximum nesting levels = " << MaxLevels << "\n"); From 468652dc34f488e5fef0a40c71e7f1d8c26f22b9 Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Wed, 26 Feb 2025 20:49:10 +0000 Subject: [PATCH 06/24] add more comments and examples --- llvm/lib/Analysis/DependenceAnalysis.cpp | 41 ++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp index 4a730fdf45f86..96f3dc5eca96c 100644 --- a/llvm/lib/Analysis/DependenceAnalysis.cpp +++ b/llvm/lib/Analysis/DependenceAnalysis.cpp @@ -3569,6 +3569,11 @@ bool DependenceInfo::invalidate(Function &F, const PreservedAnalyses &PA, Inv.invalidate(F, PA); } +// Check that memory access offsets in V are multiples of array element size +// EltSize. Param records the first parametric expression. If the scalar +// evolution V contains two or more parameters, we check that the subsequent +// parametric expressions are multiples of the first parametric expression +// Param. static bool checkOffsets(ScalarEvolution *SE, const SCEV *V, const SCEV *&Param, uint64_t EltSize) { if (auto *AddRec = dyn_cast(V)) { @@ -3578,9 +3583,20 @@ static bool checkOffsets(ScalarEvolution *SE, const SCEV *V, const SCEV *&Param, } if (auto *Cst = dyn_cast(V)) { APInt C = Cst->getAPInt(); + + // For example, alias_with_different_offsets in + // test/Analysis/DependenceAnalysis/DifferentOffsets.ll accesses "%A + 2": + // %arrayidx = getelementptr inbounds i8, ptr %A, i64 2 + // store i32 42, ptr %arrayidx, align 1 + // which is writing an i32, i.e., EltSize = 4 bytes, with an offset C = 2. + // checkOffsets returns false, as the offset C=2 is not a multiple of 4. return C.srem(EltSize) == 0; } + // Use a lambda helper function to check V for parametric expressions. + // Param records the first parametric expression. If the scalar evolution V + // contains two or more parameters, we check that the subsequent parametric + // expressions are multiples of the first parametric expression Param. auto checkParamsMultipleOfSize = [&](const SCEV *V, const SCEV *&Param) -> bool { if (EltSize == 1) @@ -3600,6 +3616,12 @@ static bool checkOffsets(ScalarEvolution *SE, const SCEV *V, const SCEV *&Param, return true; auto Rem = Val.srem(APInt(Val.getBitWidth(), EltSize, /*isSigned=*/true)); if (Rem.isZero()) + // For example in test/Analysis/DependenceAnalysis/Preliminary.ll + // SrcSCEV = ((4 * (sext i8 %n to i64)) + %A) + // DstSCEV = (4 + (4 * (sext i8 %n to i64)) + %A) + // Param = (4 * (sext i8 %n to i64)) + // V = 4 + (4 * (sext i8 %n to i64)) + // Diff = -4, Rem = 0, and so all offsets are multiple of 4. return true; LLVM_DEBUG(dbgs() << "SCEV with different offsets: " << *Param << " - " << *V << " = " << *Diff << " % " << EltSize << " = " @@ -3614,6 +3636,10 @@ static bool checkOffsets(ScalarEvolution *SE, const SCEV *V, const SCEV *&Param, const SCEV *Remainder = SE->getURemExpr(Diff, Val); if (const SCEVConstant *C = dyn_cast(Remainder)) if (C->getValue()->isZero()) + // For example test/Analysis/DependenceAnalysis/DADelin.ll + // SrcSCEV = {{{%A,+,(4 * %m * %o)}<%for.cond1.preheader>,+,(4 * %o)} + // DstSCEV = {{{%A,+,(4 * %m * %o)}<%for.cond1.preheader>,+,(4 * %o)} + // The strides '(4 * %m * %o)' and '(4 * %o)' are multiple of 4. return true; // Check by using the division computation. @@ -3625,17 +3651,32 @@ static bool checkOffsets(ScalarEvolution *SE, const SCEV *V, const SCEV *&Param, << *Param << " - " << *V << " = " << *Diff << "\n" << "Remainder = " << *Remainder << "\n" << "Q = " << *Q << " Product = " << *Product << "\n"); + // For example in test/Analysis/DependenceAnalysis/MIVCheckConst.ll + // SrcSCEV = {(80640 + (4 * (1 + %n) * %v1) + %A),+,(8 * %v1)}<%bb13> + // DstSCEV = {(126720 + (128 * %m) + %A),+,256}<%bb13> + // We fail to prove that the offsets 80640 + (4 * (1 + %n) * %v1) and + // (8 * %v1) are multiples of 128. + // Param = 80640 + (4 * (1 + %n) * %v1) + // (80640 + (4 * (1 + %n) * %v1)) - (8 * %v1) = + // (80640 + ((-4 + (4 * %n)) * %v1)) + // Remainder = (zext i7 ((trunc i32 %v1 to i7) * + // (-4 + (4 * (trunc i32 %n to i7)))) to i32) + // Q = ((80640 + ((-4 + (4 * %n)) * %v1)) /u 128) + // Product = (128 * ((80640 + ((-4 + (4 * %n)) * %v1)) /u 128)) return false; }; + // Expressions like "n". if (isa(V)) return checkParamsMultipleOfSize(V, Param); + // Expressions like "n + 1". if (isa(V)) return !SCEVExprContains(V, [](const SCEV *S) { return isa(S); }) || checkParamsMultipleOfSize(V, Param); + // Expressions like "n * 2". if (isa(V)) return !SCEVExprContains(V, [](const SCEV *S) { return isa(S); From 108d2247e3ad2e9ba13f731ebf2833eeb8c7b76f Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Thu, 27 Feb 2025 11:51:05 -0600 Subject: [PATCH 07/24] add extra slash for function comments Co-authored-by: Michael Kruse --- llvm/lib/Analysis/DependenceAnalysis.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp index 96f3dc5eca96c..4c3357f7543d3 100644 --- a/llvm/lib/Analysis/DependenceAnalysis.cpp +++ b/llvm/lib/Analysis/DependenceAnalysis.cpp @@ -3569,11 +3569,11 @@ bool DependenceInfo::invalidate(Function &F, const PreservedAnalyses &PA, Inv.invalidate(F, PA); } -// Check that memory access offsets in V are multiples of array element size -// EltSize. Param records the first parametric expression. If the scalar -// evolution V contains two or more parameters, we check that the subsequent -// parametric expressions are multiples of the first parametric expression -// Param. +/// Check that memory access offsets in V are multiples of array element size +/// EltSize. Param records the first parametric expression. If the scalar +/// evolution V contains two or more parameters, we check that the subsequent +/// parametric expressions are multiples of the first parametric expression +/// Param. static bool checkOffsets(ScalarEvolution *SE, const SCEV *V, const SCEV *&Param, uint64_t EltSize) { if (auto *AddRec = dyn_cast(V)) { From c62e1f424b253e34b61cbbc1778897a6186d20b8 Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Thu, 27 Feb 2025 11:52:37 -0600 Subject: [PATCH 08/24] simplify logic with a single return Co-authored-by: Michael Kruse --- llvm/lib/Analysis/DependenceAnalysis.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp index 4c3357f7543d3..82b271152f614 100644 --- a/llvm/lib/Analysis/DependenceAnalysis.cpp +++ b/llvm/lib/Analysis/DependenceAnalysis.cpp @@ -3577,9 +3577,8 @@ bool DependenceInfo::invalidate(Function &F, const PreservedAnalyses &PA, static bool checkOffsets(ScalarEvolution *SE, const SCEV *V, const SCEV *&Param, uint64_t EltSize) { if (auto *AddRec = dyn_cast(V)) { - if (!checkOffsets(SE, AddRec->getStart(), Param, EltSize)) - return false; - return checkOffsets(SE, AddRec->getStepRecurrence(*SE), Param, EltSize); + return checkOffsets(SE, AddRec->getStart(), Param, EltSize) && + checkOffsets(SE, AddRec->getStepRecurrence(*SE), Param, EltSize); } if (auto *Cst = dyn_cast(V)) { APInt C = Cst->getAPInt(); From 8dcc5a0d2f43f182794463d2ac5847a7286c7076 Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Thu, 27 Feb 2025 11:54:31 -0600 Subject: [PATCH 09/24] use auto on dyn_cast assign for readability Co-authored-by: Michael Kruse --- llvm/lib/Analysis/DependenceAnalysis.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp index 82b271152f614..5c5fca3b3fed9 100644 --- a/llvm/lib/Analysis/DependenceAnalysis.cpp +++ b/llvm/lib/Analysis/DependenceAnalysis.cpp @@ -3609,7 +3609,7 @@ static bool checkOffsets(ScalarEvolution *SE, const SCEV *V, const SCEV *&Param, // Check whether "(Param - V) % Size == 0". const SCEV *Diff = SE->getMinusSCEV(Param, V); - if (const SCEVConstant *Cst = dyn_cast(Diff)) { + if (auto *Cst = dyn_cast(Diff)) { APInt Val = Cst->getAPInt(); if (Val.isZero()) return true; From 92f6b4fddf2db8c693480b43dda37b1d75034d20 Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Thu, 27 Feb 2025 11:56:12 -0600 Subject: [PATCH 10/24] simplify logic for similar code-paths Co-authored-by: Michael Kruse --- llvm/lib/Analysis/DependenceAnalysis.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp index 5c5fca3b3fed9..5d1f58253cfc0 100644 --- a/llvm/lib/Analysis/DependenceAnalysis.cpp +++ b/llvm/lib/Analysis/DependenceAnalysis.cpp @@ -3670,13 +3670,7 @@ static bool checkOffsets(ScalarEvolution *SE, const SCEV *V, const SCEV *&Param, return checkParamsMultipleOfSize(V, Param); // Expressions like "n + 1". - if (isa(V)) - return !SCEVExprContains(V, [](const SCEV *S) { - return isa(S); - }) || checkParamsMultipleOfSize(V, Param); - - // Expressions like "n * 2". - if (isa(V)) + if (isa(V) || isa(V)) return !SCEVExprContains(V, [](const SCEV *S) { return isa(S); }) || checkParamsMultipleOfSize(V, Param); From 0eae7f08c0c66d2751036c7adf4c39de748df918 Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Thu, 27 Feb 2025 22:24:46 +0000 Subject: [PATCH 11/24] move isKnownMultipleOf to ScalarEvolution --- llvm/include/llvm/Analysis/ScalarEvolution.h | 7 ++ llvm/lib/Analysis/DependenceAnalysis.cpp | 114 +------------------ llvm/lib/Analysis/ScalarEvolution.cpp | 104 +++++++++++++++++ 3 files changed, 113 insertions(+), 112 deletions(-) diff --git a/llvm/include/llvm/Analysis/ScalarEvolution.h b/llvm/include/llvm/Analysis/ScalarEvolution.h index f729b07076d29..d6628f22ee7bb 100644 --- a/llvm/include/llvm/Analysis/ScalarEvolution.h +++ b/llvm/include/llvm/Analysis/ScalarEvolution.h @@ -1044,6 +1044,13 @@ class ScalarEvolution { bool isKnownToBeAPowerOfTwo(const SCEV *S, bool OrZero = false, bool OrNegative = false); + /// Check that memory access offsets in V are multiples of array element size + /// EltSize. Param records the first parametric expression. If the scalar + /// evolution V contains two or more parameters, we check that the subsequent + /// parametric expressions are multiples of the first parametric expression + /// Param. + bool isKnownMultipleOf(const SCEV *V, const SCEV *&Param, uint64_t EltSize); + /// Splits SCEV expression \p S into two SCEVs. One of them is obtained from /// \p S by substitution of all AddRec sub-expression related to loop \p L /// with initial value of that SCEV. The second is obtained from \p S by diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp index 5d1f58253cfc0..90cf533252cd3 100644 --- a/llvm/lib/Analysis/DependenceAnalysis.cpp +++ b/llvm/lib/Analysis/DependenceAnalysis.cpp @@ -3569,116 +3569,6 @@ bool DependenceInfo::invalidate(Function &F, const PreservedAnalyses &PA, Inv.invalidate(F, PA); } -/// Check that memory access offsets in V are multiples of array element size -/// EltSize. Param records the first parametric expression. If the scalar -/// evolution V contains two or more parameters, we check that the subsequent -/// parametric expressions are multiples of the first parametric expression -/// Param. -static bool checkOffsets(ScalarEvolution *SE, const SCEV *V, const SCEV *&Param, - uint64_t EltSize) { - if (auto *AddRec = dyn_cast(V)) { - return checkOffsets(SE, AddRec->getStart(), Param, EltSize) && - checkOffsets(SE, AddRec->getStepRecurrence(*SE), Param, EltSize); - } - if (auto *Cst = dyn_cast(V)) { - APInt C = Cst->getAPInt(); - - // For example, alias_with_different_offsets in - // test/Analysis/DependenceAnalysis/DifferentOffsets.ll accesses "%A + 2": - // %arrayidx = getelementptr inbounds i8, ptr %A, i64 2 - // store i32 42, ptr %arrayidx, align 1 - // which is writing an i32, i.e., EltSize = 4 bytes, with an offset C = 2. - // checkOffsets returns false, as the offset C=2 is not a multiple of 4. - return C.srem(EltSize) == 0; - } - - // Use a lambda helper function to check V for parametric expressions. - // Param records the first parametric expression. If the scalar evolution V - // contains two or more parameters, we check that the subsequent parametric - // expressions are multiples of the first parametric expression Param. - auto checkParamsMultipleOfSize = [&](const SCEV *V, - const SCEV *&Param) -> bool { - if (EltSize == 1) - return true; - if (!Param) { - Param = V; - return true; - } - if (Param == V) - return true; - - // Check whether "(Param - V) % Size == 0". - const SCEV *Diff = SE->getMinusSCEV(Param, V); - if (auto *Cst = dyn_cast(Diff)) { - APInt Val = Cst->getAPInt(); - if (Val.isZero()) - return true; - auto Rem = Val.srem(APInt(Val.getBitWidth(), EltSize, /*isSigned=*/true)); - if (Rem.isZero()) - // For example in test/Analysis/DependenceAnalysis/Preliminary.ll - // SrcSCEV = ((4 * (sext i8 %n to i64)) + %A) - // DstSCEV = (4 + (4 * (sext i8 %n to i64)) + %A) - // Param = (4 * (sext i8 %n to i64)) - // V = 4 + (4 * (sext i8 %n to i64)) - // Diff = -4, Rem = 0, and so all offsets are multiple of 4. - return true; - LLVM_DEBUG(dbgs() << "SCEV with different offsets: " << *Param << " - " - << *V << " = " << *Diff << " % " << EltSize << " = " - << Rem << " != 0\n"); - return false; - } - // Check if the symbolic difference is a multiple of Size. - const SCEV *Val = - SE->getConstant(APInt(Diff->getType()->getScalarSizeInBits(), EltSize)); - - // Check by using the remainder computation. - const SCEV *Remainder = SE->getURemExpr(Diff, Val); - if (const SCEVConstant *C = dyn_cast(Remainder)) - if (C->getValue()->isZero()) - // For example test/Analysis/DependenceAnalysis/DADelin.ll - // SrcSCEV = {{{%A,+,(4 * %m * %o)}<%for.cond1.preheader>,+,(4 * %o)} - // DstSCEV = {{{%A,+,(4 * %m * %o)}<%for.cond1.preheader>,+,(4 * %o)} - // The strides '(4 * %m * %o)' and '(4 * %o)' are multiple of 4. - return true; - - // Check by using the division computation. - const SCEV *Q = SE->getUDivExpr(Diff, Val); - const SCEV *Product = SE->getMulExpr(Q, Val); - if (Diff == Product) - return true; - LLVM_DEBUG(dbgs() << "SCEV with different offsets:\n" - << *Param << " - " << *V << " = " << *Diff << "\n" - << "Remainder = " << *Remainder << "\n" - << "Q = " << *Q << " Product = " << *Product << "\n"); - // For example in test/Analysis/DependenceAnalysis/MIVCheckConst.ll - // SrcSCEV = {(80640 + (4 * (1 + %n) * %v1) + %A),+,(8 * %v1)}<%bb13> - // DstSCEV = {(126720 + (128 * %m) + %A),+,256}<%bb13> - // We fail to prove that the offsets 80640 + (4 * (1 + %n) * %v1) and - // (8 * %v1) are multiples of 128. - // Param = 80640 + (4 * (1 + %n) * %v1) - // (80640 + (4 * (1 + %n) * %v1)) - (8 * %v1) = - // (80640 + ((-4 + (4 * %n)) * %v1)) - // Remainder = (zext i7 ((trunc i32 %v1 to i7) * - // (-4 + (4 * (trunc i32 %n to i7)))) to i32) - // Q = ((80640 + ((-4 + (4 * %n)) * %v1)) /u 128) - // Product = (128 * ((80640 + ((-4 + (4 * %n)) * %v1)) /u 128)) - return false; - }; - - // Expressions like "n". - if (isa(V)) - return checkParamsMultipleOfSize(V, Param); - - // Expressions like "n + 1". - if (isa(V) || isa(V)) - return !SCEVExprContains(V, [](const SCEV *S) { - return isa(S); - }) || checkParamsMultipleOfSize(V, Param); - - LLVM_DEBUG(dbgs() << "SCEV node not handled yet: " << *V << "\n"); - return false; -} - // depends - // Returns NULL if there is no dependence. // Otherwise, return a Dependence with as many details as possible. @@ -3758,8 +3648,8 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst) { if (Src != Dst) { // Check that memory access offsets are multiples of element sizes. - if (!checkOffsets(SE, SrcEv, Param, EltSize) || - !checkOffsets(SE, DstEv, Param, EltSize)) { + if (!SE->isKnownMultipleOf(SrcEv, Param, EltSize) || + !SE->isKnownMultipleOf(DstEv, Param, EltSize)) { LLVM_DEBUG(dbgs() << "can't analyze SCEV with different offsets\n"); return std::make_unique(Src, Dst); } diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp index c62ea1526981d..2a3f4692ba6cc 100644 --- a/llvm/lib/Analysis/ScalarEvolution.cpp +++ b/llvm/lib/Analysis/ScalarEvolution.cpp @@ -10971,6 +10971,110 @@ bool ScalarEvolution::isKnownToBeAPowerOfTwo(const SCEV *S, bool OrZero, return all_of(Mul->operands(), NonRecursive) && (OrZero || isKnownNonZero(S)); } +bool ScalarEvolution::isKnownMultipleOf(const SCEV *V, const SCEV *&Param, + uint64_t EltSize) { + if (auto *AddRec = dyn_cast(V)) + return isKnownMultipleOf(AddRec->getStart(), Param, EltSize) && + isKnownMultipleOf(AddRec->getStepRecurrence(*this), Param, EltSize); + + if (auto *Cst = dyn_cast(V)) { + APInt C = Cst->getAPInt(); + // For example, alias_with_different_offsets in + // test/Analysis/DependenceAnalysis/DifferentOffsets.ll accesses "%A + 2": + // %arrayidx = getelementptr inbounds i8, ptr %A, i64 2 + // store i32 42, ptr %arrayidx, align 1 + // which is writing an i32, i.e., EltSize = 4 bytes, with an offset C = 2. + // isKnownMultipleOf returns false, as C=2 is not a multiple of 4. + return C.urem(EltSize) == 0; + } + + // Use a lambda helper function to check V for parametric expressions. + // Param records the first parametric expression. If the scalar evolution V + // contains two or more parameters, we check that the subsequent parametric + // expressions are multiples of the first parametric expression Param. + auto checkParamsMultipleOfSize = [&](const SCEV *V, + const SCEV *&Param) -> bool { + if (EltSize == 1) + return true; + if (!Param) { + Param = V; + return true; + } + if (Param == V) + return true; + + // Check whether "(Param - V) % Size == 0". + const SCEV *Diff = getMinusSCEV(Param, V); + if (auto *Cst = dyn_cast(Diff)) { + APInt Val = Cst->getAPInt(); + if (Val.isZero()) + return true; + APInt Rem = Val.urem(APInt(Val.getBitWidth(), EltSize, /*isSigned=*/true)); + if (Rem.isZero()) + // For example in test/Analysis/DependenceAnalysis/Preliminary.ll + // SrcSCEV = ((4 * (sext i8 %n to i64)) + %A) + // DstSCEV = (4 + (4 * (sext i8 %n to i64)) + %A) + // Param = (4 * (sext i8 %n to i64)) + // V = 4 + (4 * (sext i8 %n to i64)) + // Diff = -4, Rem = 0, and so all offsets are multiple of 4. + return true; + LLVM_DEBUG(dbgs() << "SCEV with different offsets: " << *Param << " - " + << *V << " = " << *Diff << " % " << EltSize << " = " + << Rem << " != 0\n"); + return false; + } + // Check if the symbolic difference is a multiple of Size. + const SCEV *Val = + getConstant(APInt(Diff->getType()->getScalarSizeInBits(), EltSize)); + + // Check by using the remainder computation. + const SCEV *Remainder = getURemExpr(Diff, Val); + if (const SCEVConstant *C = dyn_cast(Remainder)) + if (C->getValue()->isZero()) + // For example test/Analysis/DependenceAnalysis/DADelin.ll + // SrcSCEV = {{{%A,+,(4 * %m * %o)}<%for.cond1.preheader>,+,(4 * %o)} + // DstSCEV = {{{%A,+,(4 * %m * %o)}<%for.cond1.preheader>,+,(4 * %o)} + // The strides '(4 * %m * %o)' and '(4 * %o)' are multiple of 4. + return true; + + // Check by using the division computation. + const SCEV *Q = getUDivExpr(Diff, Val); + const SCEV *Product = getMulExpr(Q, Val); + if (Diff == Product) + return true; + LLVM_DEBUG(dbgs() << "SCEV with different offsets:\n" + << *Param << " - " << *V << " = " << *Diff << "\n" + << "Remainder = " << *Remainder << "\n" + << "Q = " << *Q << " Product = " << *Product << "\n"); + // For example in test/Analysis/DependenceAnalysis/MIVCheckConst.ll + // SrcSCEV = {(80640 + (4 * (1 + %n) * %v1) + %A),+,(8 * %v1)}<%bb13> + // DstSCEV = {(126720 + (128 * %m) + %A),+,256}<%bb13> + // We fail to prove that the offsets 80640 + (4 * (1 + %n) * %v1) and + // (8 * %v1) are multiples of 128. + // Param = 80640 + (4 * (1 + %n) * %v1) + // (80640 + (4 * (1 + %n) * %v1)) - (8 * %v1) = + // (80640 + ((-4 + (4 * %n)) * %v1)) + // Remainder = (zext i7 ((trunc i32 %v1 to i7) * + // (-4 + (4 * (trunc i32 %n to i7)))) to i32) + // Q = ((80640 + ((-4 + (4 * %n)) * %v1)) /u 128) + // Product = (128 * ((80640 + ((-4 + (4 * %n)) * %v1)) /u 128)) + return false; + }; + + // Expressions like "n". + if (isa(V)) + return checkParamsMultipleOfSize(V, Param); + + // Expressions like "n + 1". + if (isa(V) || isa(V)) + return !SCEVExprContains(V, [](const SCEV *S) { + return isa(S); + }) || checkParamsMultipleOfSize(V, Param); + + LLVM_DEBUG(dbgs() << "SCEV node not handled yet: " << *V << "\n"); + return false; +} + std::pair ScalarEvolution::SplitIntoInitAndPostInc(const Loop *L, const SCEV *S) { // Compute SCEV on entry of loop L. From 60ba4e688105c7663b8bb01eed7076f77b385935 Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Tue, 8 Apr 2025 19:31:36 +0000 Subject: [PATCH 12/24] record runtime assumptions for parametric expressions --- .../llvm/Analysis/DependenceAnalysis.h | 4 + llvm/include/llvm/Analysis/ScalarEvolution.h | 10 +- llvm/lib/Analysis/DependenceAnalysis.cpp | 14 +- llvm/lib/Analysis/ScalarEvolution.cpp | 141 ++++++------------ .../DependenceAnalysis/DifferentOffsets.ll | 42 ++++++ .../DependenceAnalysis/MIVCheckConst.ll | 5 +- 6 files changed, 114 insertions(+), 102 deletions(-) diff --git a/llvm/include/llvm/Analysis/DependenceAnalysis.h b/llvm/include/llvm/Analysis/DependenceAnalysis.h index 426ac757b4b0d..6ba3952e6f838 100644 --- a/llvm/include/llvm/Analysis/DependenceAnalysis.h +++ b/llvm/include/llvm/Analysis/DependenceAnalysis.h @@ -52,6 +52,8 @@ namespace llvm { class ScalarEvolution; class SCEV; class SCEVConstant; + class SCEVPredicate; + class SCEVUnionPredicate; class raw_ostream; /// Dependence - This class represents a dependence between two memory @@ -349,12 +351,14 @@ namespace llvm { const SCEV *getSplitIteration(const Dependence &Dep, unsigned Level); Function *getFunction() const { return F; } + SCEVUnionPredicate getRuntimeAssumptions(); private: AAResults *AA; ScalarEvolution *SE; LoopInfo *LI; Function *F; + SmallVector Assumptions; /// Subscript - This private struct represents a pair of subscripts from /// a pair of potentially multi-dimensional array references. We use a diff --git a/llvm/include/llvm/Analysis/ScalarEvolution.h b/llvm/include/llvm/Analysis/ScalarEvolution.h index d6628f22ee7bb..eadc6752a323d 100644 --- a/llvm/include/llvm/Analysis/ScalarEvolution.h +++ b/llvm/include/llvm/Analysis/ScalarEvolution.h @@ -1044,12 +1044,10 @@ class ScalarEvolution { bool isKnownToBeAPowerOfTwo(const SCEV *S, bool OrZero = false, bool OrNegative = false); - /// Check that memory access offsets in V are multiples of array element size - /// EltSize. Param records the first parametric expression. If the scalar - /// evolution V contains two or more parameters, we check that the subsequent - /// parametric expressions are multiples of the first parametric expression - /// Param. - bool isKnownMultipleOf(const SCEV *V, const SCEV *&Param, uint64_t EltSize); + /// Check that memory access offsets in S are multiples of M. Assumptions + /// records the runtime predicates under which S is a multiple of M. + bool isKnownMultipleOf(const SCEV *S, uint64_t M, + SmallVectorImpl &Assumptions); /// Splits SCEV expression \p S into two SCEVs. One of them is obtained from /// \p S by substitution of all AddRec sub-expression related to loop \p L diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp index 90cf533252cd3..ffbc777e173dc 100644 --- a/llvm/lib/Analysis/DependenceAnalysis.cpp +++ b/llvm/lib/Analysis/DependenceAnalysis.cpp @@ -206,6 +206,11 @@ static void dumpExampleDependence(raw_ostream &OS, DependenceInfo *DA, } } } + SCEVUnionPredicate Assumptions = DA->getRuntimeAssumptions(); + if (!Assumptions.isAlwaysTrue()) { + OS << "Runtime Assumptions:\n"; + Assumptions.print(OS, 0); + } } void DependenceAnalysisWrapperPass::print(raw_ostream &OS, @@ -3569,6 +3574,10 @@ bool DependenceInfo::invalidate(Function &F, const PreservedAnalyses &PA, Inv.invalidate(F, PA); } +SCEVUnionPredicate DependenceInfo::getRuntimeAssumptions() { + return SCEVUnionPredicate(Assumptions, *SE); +} + // depends - // Returns NULL if there is no dependence. // Otherwise, return a Dependence with as many details as possible. @@ -3644,12 +3653,11 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst) { const SCEV *SrcEv = SE->getMinusSCEV(SrcSCEV, SrcBase); const SCEV *DstEv = SE->getMinusSCEV(DstSCEV, DstBase); - const SCEV *Param = nullptr; if (Src != Dst) { // Check that memory access offsets are multiples of element sizes. - if (!SE->isKnownMultipleOf(SrcEv, Param, EltSize) || - !SE->isKnownMultipleOf(DstEv, Param, EltSize)) { + if (!SE->isKnownMultipleOf(SrcEv, EltSize, Assumptions) || + !SE->isKnownMultipleOf(DstEv, EltSize, Assumptions)) { LLVM_DEBUG(dbgs() << "can't analyze SCEV with different offsets\n"); return std::make_unique(Src, Dst); } diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp index 2a3f4692ba6cc..c0c426ff95203 100644 --- a/llvm/lib/Analysis/ScalarEvolution.cpp +++ b/llvm/lib/Analysis/ScalarEvolution.cpp @@ -10971,107 +10971,64 @@ bool ScalarEvolution::isKnownToBeAPowerOfTwo(const SCEV *S, bool OrZero, return all_of(Mul->operands(), NonRecursive) && (OrZero || isKnownNonZero(S)); } -bool ScalarEvolution::isKnownMultipleOf(const SCEV *V, const SCEV *&Param, - uint64_t EltSize) { - if (auto *AddRec = dyn_cast(V)) - return isKnownMultipleOf(AddRec->getStart(), Param, EltSize) && - isKnownMultipleOf(AddRec->getStepRecurrence(*this), Param, EltSize); +bool ScalarEvolution::isKnownMultipleOf( + const SCEV *S, uint64_t M, + SmallVectorImpl &Assumptions) { + if (M == 0) + return false; + if (M == 1) + return true; + + // Recursively check AddRec operands. + if (auto *AddRec = dyn_cast(S)) + return isKnownMultipleOf(AddRec->getStart(), M, Assumptions) && + isKnownMultipleOf(AddRec->getStepRecurrence(*this), M, Assumptions); - if (auto *Cst = dyn_cast(V)) { + // For a constant, check that "S % M == 0". + if (auto *Cst = dyn_cast(S)) { APInt C = Cst->getAPInt(); - // For example, alias_with_different_offsets in - // test/Analysis/DependenceAnalysis/DifferentOffsets.ll accesses "%A + 2": - // %arrayidx = getelementptr inbounds i8, ptr %A, i64 2 - // store i32 42, ptr %arrayidx, align 1 - // which is writing an i32, i.e., EltSize = 4 bytes, with an offset C = 2. - // isKnownMultipleOf returns false, as C=2 is not a multiple of 4. - return C.urem(EltSize) == 0; - } - - // Use a lambda helper function to check V for parametric expressions. - // Param records the first parametric expression. If the scalar evolution V - // contains two or more parameters, we check that the subsequent parametric - // expressions are multiples of the first parametric expression Param. - auto checkParamsMultipleOfSize = [&](const SCEV *V, - const SCEV *&Param) -> bool { - if (EltSize == 1) - return true; - if (!Param) { - Param = V; - return true; - } - if (Param == V) - return true; + return C.urem(M) == 0; + } - // Check whether "(Param - V) % Size == 0". - const SCEV *Diff = getMinusSCEV(Param, V); - if (auto *Cst = dyn_cast(Diff)) { - APInt Val = Cst->getAPInt(); - if (Val.isZero()) - return true; - APInt Rem = Val.urem(APInt(Val.getBitWidth(), EltSize, /*isSigned=*/true)); - if (Rem.isZero()) - // For example in test/Analysis/DependenceAnalysis/Preliminary.ll - // SrcSCEV = ((4 * (sext i8 %n to i64)) + %A) - // DstSCEV = (4 + (4 * (sext i8 %n to i64)) + %A) - // Param = (4 * (sext i8 %n to i64)) - // V = 4 + (4 * (sext i8 %n to i64)) - // Diff = -4, Rem = 0, and so all offsets are multiple of 4. - return true; - LLVM_DEBUG(dbgs() << "SCEV with different offsets: " << *Param << " - " - << *V << " = " << *Diff << " % " << EltSize << " = " - << Rem << " != 0\n"); - return false; - } - // Check if the symbolic difference is a multiple of Size. - const SCEV *Val = - getConstant(APInt(Diff->getType()->getScalarSizeInBits(), EltSize)); - - // Check by using the remainder computation. - const SCEV *Remainder = getURemExpr(Diff, Val); - if (const SCEVConstant *C = dyn_cast(Remainder)) - if (C->getValue()->isZero()) - // For example test/Analysis/DependenceAnalysis/DADelin.ll - // SrcSCEV = {{{%A,+,(4 * %m * %o)}<%for.cond1.preheader>,+,(4 * %o)} - // DstSCEV = {{{%A,+,(4 * %m * %o)}<%for.cond1.preheader>,+,(4 * %o)} - // The strides '(4 * %m * %o)' and '(4 * %o)' are multiple of 4. - return true; + // Basic tests have failed. + // Record "S % M == 0" in the runtime Assumptions. + auto recordRuntimePredicate = [&](const SCEV *S) -> void { + auto *STy = dyn_cast(S->getType()); + const SCEV *SmodM = + getURemExpr(S, getConstant(ConstantInt::get(STy, M, false))); + const SCEV *Zero = getZero(STy); - // Check by using the division computation. - const SCEV *Q = getUDivExpr(Diff, Val); - const SCEV *Product = getMulExpr(Q, Val); - if (Diff == Product) - return true; - LLVM_DEBUG(dbgs() << "SCEV with different offsets:\n" - << *Param << " - " << *V << " = " << *Diff << "\n" - << "Remainder = " << *Remainder << "\n" - << "Q = " << *Q << " Product = " << *Product << "\n"); - // For example in test/Analysis/DependenceAnalysis/MIVCheckConst.ll - // SrcSCEV = {(80640 + (4 * (1 + %n) * %v1) + %A),+,(8 * %v1)}<%bb13> - // DstSCEV = {(126720 + (128 * %m) + %A),+,256}<%bb13> - // We fail to prove that the offsets 80640 + (4 * (1 + %n) * %v1) and - // (8 * %v1) are multiples of 128. - // Param = 80640 + (4 * (1 + %n) * %v1) - // (80640 + (4 * (1 + %n) * %v1)) - (8 * %v1) = - // (80640 + ((-4 + (4 * %n)) * %v1)) - // Remainder = (zext i7 ((trunc i32 %v1 to i7) * - // (-4 + (4 * (trunc i32 %n to i7)))) to i32) - // Q = ((80640 + ((-4 + (4 * %n)) * %v1)) /u 128) - // Product = (128 * ((80640 + ((-4 + (4 * %n)) * %v1)) /u 128)) - return false; + // Check whether "S % M == 0" is known at compile time. + if (isKnownPredicate(ICmpInst::ICMP_EQ, SmodM, Zero)) + return; + + const SCEVPredicate *P = + getComparePredicate(ICmpInst::ICMP_EQ, SmodM, Zero); + + // Detect redundant predicates. + for (auto *A : Assumptions) + if (A->implies(P, *this)) + return; + + Assumptions.push_back(P); + return; }; // Expressions like "n". - if (isa(V)) - return checkParamsMultipleOfSize(V, Param); + if (isa(S)) { + recordRuntimePredicate(S); + return true; + } - // Expressions like "n + 1". - if (isa(V) || isa(V)) - return !SCEVExprContains(V, [](const SCEV *S) { - return isa(S); - }) || checkParamsMultipleOfSize(V, Param); + // Expressions like "n + 1" and "n * 3". + if (isa(S) || isa(S)) { + if (SCEVExprContains(S, [](const SCEV *X) { return isa(X); })) + recordRuntimePredicate(S); + return true; + } - LLVM_DEBUG(dbgs() << "SCEV node not handled yet: " << *V << "\n"); + LLVM_DEBUG(dbgs() << "SCEV node not handled yet in isKnownMultipleOf: " << *S + << "\n"); return false; } diff --git a/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll b/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll index 00375da1f26de..7d61b208cde17 100644 --- a/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll +++ b/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll @@ -23,3 +23,45 @@ entry: %0 = load i32, ptr %A, align 1 ret i32 %0 } + +define i32 @alias_with_parametric_offset(ptr nocapture %A, i64 %n) { +; CHECK-LABEL: 'alias_with_parametric_offset' +; CHECK-NEXT: Src: store i32 2, ptr %arrayidx, align 1 --> Dst: store i32 2, ptr %arrayidx, align 1 +; CHECK-NEXT: da analyze - none! +; CHECK-NEXT: Src: store i32 2, ptr %arrayidx, align 1 --> Dst: %0 = load i32, ptr %A, align 1 +; CHECK-NEXT: da analyze - flow [|<]! +; CHECK-NEXT: Src: %0 = load i32, ptr %A, align 1 --> Dst: %0 = load i32, ptr %A, align 1 +; CHECK-NEXT: da analyze - none! +; CHECK-NEXT: Runtime Assumptions: +; CHECK-NEXT: Equal predicate: (zext i2 (trunc i64 %n to i2) to i64) == 0 +; +entry: + %arrayidx = getelementptr inbounds i8, ptr %A, i64 %n + store i32 2, ptr %arrayidx, align 1 + %0 = load i32, ptr %A, align 1 + ret i32 %0 +} + +define i32 @alias_with_parametric_expr(ptr nocapture %A, i64 %n, i64 %m) { +; CHECK-LABEL: 'alias_with_parametric_expr' +; CHECK-NEXT: Src: store i32 2, ptr %arrayidx, align 1 --> Dst: store i32 2, ptr %arrayidx, align 1 +; CHECK-NEXT: da analyze - none! +; CHECK-NEXT: Src: store i32 2, ptr %arrayidx, align 1 --> Dst: %0 = load i32, ptr %arrayidx1, align 1 +; CHECK-NEXT: da analyze - flow [|<]! +; CHECK-NEXT: Src: %0 = load i32, ptr %arrayidx1, align 1 --> Dst: %0 = load i32, ptr %arrayidx1, align 1 +; CHECK-NEXT: da analyze - none! +; CHECK-NEXT: Runtime Assumptions: +; CHECK-NEXT: Equal predicate: (zext i2 ((trunc i64 %m to i2) + (-2 * (trunc i64 %n to i2))) to i64) == 0 +; CHECK-NEXT: Equal predicate: (zext i2 (-2 + (trunc i64 %m to i2)) to i64) == 0 +; +entry: + %mul = mul nsw i64 %n, 10 + %add = add nsw i64 %mul, %m + %arrayidx = getelementptr inbounds i8, ptr %A, i64 %add + store i32 2, ptr %arrayidx, align 1 + + %add1 = add nsw i64 %m, 42 + %arrayidx1 = getelementptr inbounds i8, ptr %A, i64 %add1 + %0 = load i32, ptr %arrayidx1, align 1 + ret i32 %0 +} diff --git a/llvm/test/Analysis/DependenceAnalysis/MIVCheckConst.ll b/llvm/test/Analysis/DependenceAnalysis/MIVCheckConst.ll index c8fbbe60b0bf2..af814684b37cd 100644 --- a/llvm/test/Analysis/DependenceAnalysis/MIVCheckConst.ll +++ b/llvm/test/Analysis/DependenceAnalysis/MIVCheckConst.ll @@ -41,9 +41,12 @@ define void @test(ptr %A, ptr %B, i1 %arg, i32 %n, i32 %m) #0 align 2 { ; CHECK-NEXT: Src: %v27 = load <32 x i32>, ptr %v25, align 256 --> Dst: %v27 = load <32 x i32>, ptr %v25, align 256 ; CHECK-NEXT: da analyze - consistent input [0 S S]! ; CHECK-NEXT: Src: %v27 = load <32 x i32>, ptr %v25, align 256 --> Dst: %v32 = load <32 x i32>, ptr %v30, align 128 -; CHECK-NEXT: da analyze - confused! +; CHECK-NEXT: da analyze - input [* S S|<]! ; CHECK-NEXT: Src: %v32 = load <32 x i32>, ptr %v30, align 128 --> Dst: %v32 = load <32 x i32>, ptr %v30, align 128 ; CHECK-NEXT: da analyze - consistent input [0 S S]! +; CHECK-NEXT: Runtime Assumptions: +; CHECK-NEXT: Equal predicate: (zext i7 (4 * (trunc i32 %v1 to i7) * (1 + (trunc i32 %n to i7))) to i32) == 0 +; CHECK-NEXT: Equal predicate: (8 * (zext i4 (trunc i32 %v1 to i4) to i32)) == 0 ; entry: %v1 = load i32, ptr %B, align 4 From 8c69ebf95b860081c084b542f107ccbc000afc8f Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Wed, 9 Apr 2025 00:07:21 +0000 Subject: [PATCH 13/24] disable loop interchange, fusion, and unroll-and-jam on runtime assumptions --- llvm/lib/Transforms/Scalar/LoopFuse.cpp | 18 +++++++++++++++--- llvm/lib/Transforms/Scalar/LoopInterchange.cpp | 12 +++++++++++- llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp | 7 ++++++- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/LoopFuse.cpp b/llvm/lib/Transforms/Scalar/LoopFuse.cpp index 5bba3016ba4a1..e91217302e5e5 100644 --- a/llvm/lib/Transforms/Scalar/LoopFuse.cpp +++ b/llvm/lib/Transforms/Scalar/LoopFuse.cpp @@ -1129,7 +1129,11 @@ struct LoopFuser { } } } - return true; + SCEVUnionPredicate Assumptions = DI.getRuntimeAssumptions(); + // Fail if the dependence analysis has runtime assumptions. + // FIXME: do loop versioning to keep the original loop, and transform the + // loop under the runtime assumptions. + return Assumptions.isAlwaysTrue(); } // Returns true if the instruction \p I can be sunk to the top of the exit @@ -1172,7 +1176,11 @@ struct LoopFuser { } } - return true; + SCEVUnionPredicate Assumptions = DI.getRuntimeAssumptions(); + // Fail if the dependence analysis has runtime assumptions. + // FIXME: do loop versioning to keep the original loop, and transform the + // loop under the runtime assumptions. + return Assumptions.isAlwaysTrue(); } /// Collect instructions in the \p FC1 Preheader that can be hoisted @@ -1420,7 +1428,11 @@ struct LoopFuser { return false; } - return true; + SCEVUnionPredicate Assumptions = DI.getRuntimeAssumptions(); + // Fail if the dependence analysis has runtime assumptions. + // FIXME: do loop versioning to keep the original loop, and transform the + // loop under the runtime assumptions. + return Assumptions.isAlwaysTrue(); } /// Determine if two fusion candidates are adjacent in the CFG. diff --git a/llvm/lib/Transforms/Scalar/LoopInterchange.cpp b/llvm/lib/Transforms/Scalar/LoopInterchange.cpp index 1dccba4cfa7b8..0b618fcdc37c9 100644 --- a/llvm/lib/Transforms/Scalar/LoopInterchange.cpp +++ b/llvm/lib/Transforms/Scalar/LoopInterchange.cpp @@ -220,7 +220,11 @@ static bool populateDependencyMatrix(CharMatrix &DepMatrix, unsigned Level, } } - return true; + SCEVUnionPredicate Assumptions = DI->getRuntimeAssumptions(); + // Fail if the dependence analysis has runtime assumptions. + // FIXME: do loop versioning to keep the original loop, and transform the + // loop under the runtime assumptions. + return Assumptions.isAlwaysTrue(); } // A loop is moved from index 'from' to an index 'to'. Update the Dependence @@ -1834,6 +1838,12 @@ PreservedAnalyses LoopInterchangePass::run(LoopNest &LN, std::unique_ptr CC = CacheCost::getCacheCost(LN.getOutermostLoop(), AR, DI); + SCEVUnionPredicate Assumptions = DI.getRuntimeAssumptions(); + // Early fail when the dependence analysis has runtime assumptions. + // FIXME: this could be handled by versioning the loop. + if (!Assumptions.isAlwaysTrue()) + return PreservedAnalyses::all(); + if (!LoopInterchange(&AR.SE, &AR.LI, &DI, &AR.DT, CC, &ORE).run(LN)) return PreservedAnalyses::all(); U.markLoopNestChanged(true); diff --git a/llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp b/llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp index ca90bb65f5708..d501cb401cb4d 100644 --- a/llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp +++ b/llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp @@ -800,7 +800,12 @@ checkDependencies(Loop &Root, const BasicBlockSet &SubLoopBlocks, EarlierLoadsAndStores.append(CurrentLoadsAndStores.begin(), CurrentLoadsAndStores.end()); } - return true; + + SCEVUnionPredicate Assumptions = DI.getRuntimeAssumptions(); + // Fail if the dependence analysis has runtime assumptions. + // FIXME: do loop versioning to keep the original loop, and transform the + // loop under the runtime assumptions. + return Assumptions.isAlwaysTrue(); } static bool isEligibleLoopForm(const Loop &Root) { From a80c878342b4f02573cfef8ea43c9ecfce1dc263 Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Mon, 14 Apr 2025 21:24:02 +0000 Subject: [PATCH 14/24] handle compile time "s % m != 0" in isKnownMultipleOf Do not check for symbolic names, just handle all possible SCEV expressions with compile time tests and record runtime predicates when compile time fails. --- llvm/lib/Analysis/ScalarEvolution.cpp | 55 ++++++++++----------------- 1 file changed, 20 insertions(+), 35 deletions(-) diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp index c0c426ff95203..ef80199b68c61 100644 --- a/llvm/lib/Analysis/ScalarEvolution.cpp +++ b/llvm/lib/Analysis/ScalarEvolution.cpp @@ -10991,45 +10991,30 @@ bool ScalarEvolution::isKnownMultipleOf( } // Basic tests have failed. - // Record "S % M == 0" in the runtime Assumptions. - auto recordRuntimePredicate = [&](const SCEV *S) -> void { - auto *STy = dyn_cast(S->getType()); - const SCEV *SmodM = - getURemExpr(S, getConstant(ConstantInt::get(STy, M, false))); - const SCEV *Zero = getZero(STy); - - // Check whether "S % M == 0" is known at compile time. - if (isKnownPredicate(ICmpInst::ICMP_EQ, SmodM, Zero)) - return; - - const SCEVPredicate *P = - getComparePredicate(ICmpInst::ICMP_EQ, SmodM, Zero); + // Check "S % M == 0" at compile time and record runtime Assumptions. + auto *STy = dyn_cast(S->getType()); + const SCEV *SmodM = + getURemExpr(S, getConstant(ConstantInt::get(STy, M, false))); + const SCEV *Zero = getZero(STy); + + // Check whether "S % M == 0" is known at compile time. + if (isKnownPredicate(ICmpInst::ICMP_EQ, SmodM, Zero)) + return true; - // Detect redundant predicates. - for (auto *A : Assumptions) - if (A->implies(P, *this)) - return; + // Check whether "S % M != 0" is known at compile time. + if (isKnownPredicate(ICmpInst::ICMP_NE, SmodM, Zero)) + return false; - Assumptions.push_back(P); - return; - }; + const SCEVPredicate *P = getComparePredicate(ICmpInst::ICMP_EQ, SmodM, Zero); - // Expressions like "n". - if (isa(S)) { - recordRuntimePredicate(S); - return true; - } - - // Expressions like "n + 1" and "n * 3". - if (isa(S) || isa(S)) { - if (SCEVExprContains(S, [](const SCEV *X) { return isa(X); })) - recordRuntimePredicate(S); - return true; - } + // Detect redundant predicates. + for (auto *A : Assumptions) + if (A->implies(P, *this)) + return true; - LLVM_DEBUG(dbgs() << "SCEV node not handled yet in isKnownMultipleOf: " << *S - << "\n"); - return false; + // Only record non-redundant predicates. + Assumptions.push_back(P); + return true; } std::pair From bf9fcfda5f851c55cc89f9c75246dcb3e7bd6d98 Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Mon, 14 Apr 2025 23:51:51 +0000 Subject: [PATCH 15/24] add testcase from Ryotaro Kasuga's review --- .../DependenceAnalysis/DifferentOffsets.ll | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll b/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll index 7d61b208cde17..ba7ae7904cdbb 100644 --- a/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll +++ b/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll @@ -65,3 +65,25 @@ entry: %0 = load i32, ptr %arrayidx1, align 1 ret i32 %0 } + +define i32 @gep_i8_vs_i32(ptr nocapture %A, i64 %n, i64 %m) { +; CHECK-LABEL: 'gep_i8_vs_i32' +; CHECK-NEXT: Src: store i32 42, ptr %arrayidx0, align 1 --> Dst: store i32 42, ptr %arrayidx0, align 1 +; CHECK-NEXT: da analyze - none! +; CHECK-NEXT: Src: store i32 42, ptr %arrayidx0, align 1 --> Dst: store i32 42, ptr %arrayidx1, align 4 +; CHECK-NEXT: da analyze - output [|<]! +; CHECK-NEXT: Runtime Assumptions: +; CHECK-NEXT: Equal predicate: (zext i2 (trunc i64 %n to i2) to i64) == 0 +; CHECK-NEXT: Src: store i32 42, ptr %arrayidx1, align 4 --> Dst: store i32 42, ptr %arrayidx1, align 4 +; CHECK-NEXT: da analyze - none! +; CHECK-NEXT: Runtime Assumptions: +; CHECK-NEXT: Equal predicate: (zext i2 (trunc i64 %n to i2) to i64) == 0 +; +entry: + %arrayidx0 = getelementptr inbounds i8, ptr %A, i64 %n + store i32 42, ptr %arrayidx0, align 1 + + %arrayidx1 = getelementptr inbounds i32, ptr %A, i64 %m + store i32 42, ptr %arrayidx1, align 4 + ret i32 0 +} From 3dbba8e1ab86cc1b1b7514ad3576077f9f24dca8 Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Mon, 14 Apr 2025 21:32:52 +0000 Subject: [PATCH 16/24] record runtime predicates on each Dependence relation --- .../llvm/Analysis/DependenceAnalysis.h | 35 +++++++---- llvm/lib/Analysis/DependenceAnalysis.cpp | 59 +++++++++++++++---- .../DependenceAnalysis/DifferentOffsets.ll | 5 ++ .../DependenceAnalysis/MIVCheckConst.ll | 3 + 4 files changed, 78 insertions(+), 24 deletions(-) diff --git a/llvm/include/llvm/Analysis/DependenceAnalysis.h b/llvm/include/llvm/Analysis/DependenceAnalysis.h index 6ba3952e6f838..553bdc191bc0c 100644 --- a/llvm/include/llvm/Analysis/DependenceAnalysis.h +++ b/llvm/include/llvm/Analysis/DependenceAnalysis.h @@ -40,6 +40,7 @@ #define LLVM_ANALYSIS_DEPENDENCEANALYSIS_H #include "llvm/ADT/SmallBitVector.h" +#include "llvm/Analysis/ScalarEvolution.h" #include "llvm/IR/Instructions.h" #include "llvm/IR/PassManager.h" #include "llvm/Pass.h" @@ -49,11 +50,7 @@ namespace llvm { template class ArrayRef; class Loop; class LoopInfo; - class ScalarEvolution; - class SCEV; class SCEVConstant; - class SCEVPredicate; - class SCEVUnionPredicate; class raw_ostream; /// Dependence - This class represents a dependence between two memory @@ -76,8 +73,9 @@ namespace llvm { Dependence &operator=(Dependence &&) = default; public: - Dependence(Instruction *Source, Instruction *Destination) - : Src(Source), Dst(Destination) {} + Dependence(Instruction *Source, Instruction *Destination, + const SCEVUnionPredicate &A) + : Src(Source), Dst(Destination), Assumptions(A) {} virtual ~Dependence() = default; /// Dependence::DVEntry - Each level in the distance/direction vector @@ -205,6 +203,10 @@ namespace llvm { /// field. void setNextSuccessor(const Dependence *succ) { NextSuccessor = succ; } + /// getRuntimeAssumptions - Returns the runtime assumptions under which this + /// Dependence relation is valid. + SCEVUnionPredicate getRuntimeAssumptions() const { return Assumptions; } + /// dump - For debugging purposes, dumps a dependence to OS. /// void dump(raw_ostream &OS) const; @@ -213,6 +215,7 @@ namespace llvm { Instruction *Src, *Dst; private: + SCEVUnionPredicate Assumptions; const Dependence *NextPredecessor = nullptr, *NextSuccessor = nullptr; friend class DependenceInfo; }; @@ -227,8 +230,9 @@ namespace llvm { /// input dependences are unordered. class FullDependence final : public Dependence { public: - FullDependence(Instruction *Src, Instruction *Dst, bool LoopIndependent, - unsigned Levels); + FullDependence(Instruction *Source, Instruction *Destination, + const SCEVUnionPredicate &Assumes, + bool PossiblyLoopIndependent, unsigned Levels); /// isLoopIndependent - Returns true if this is a loop-independent /// dependence. @@ -304,9 +308,13 @@ namespace llvm { /// depends - Tests for a dependence between the Src and Dst instructions. /// Returns NULL if no dependence; otherwise, returns a Dependence (or a - /// FullDependence) with as much information as can be gleaned. - std::unique_ptr depends(Instruction *Src, - Instruction *Dst); + /// FullDependence) with as much information as can be gleaned. By default, + /// the dependence test collects a set of runtime assumptions that cannot be + /// solved at compilation time. Set UnderRuntimeAssumptions to false for a + /// safe approximation of the dependence relation that does not require + /// runtime checks. + std::unique_ptr depends(Instruction *Src, Instruction *Dst, + bool UnderRuntimeAssumptions = true); /// getSplitIteration - Give a dependence that's splittable at some /// particular level, return the iteration that should be used to split @@ -351,7 +359,10 @@ namespace llvm { const SCEV *getSplitIteration(const Dependence &Dep, unsigned Level); Function *getFunction() const { return F; } - SCEVUnionPredicate getRuntimeAssumptions(); + + /// getRuntimeAssumptions - Returns all the runtime assumptions under which + /// the dependence test is valid. + SCEVUnionPredicate getRuntimeAssumptions() const; private: AAResults *AA; diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp index ffbc777e173dc..5d17ceb9dac2c 100644 --- a/llvm/lib/Analysis/DependenceAnalysis.cpp +++ b/llvm/lib/Analysis/DependenceAnalysis.cpp @@ -269,9 +269,10 @@ bool Dependence::isScalar(unsigned level) const { // FullDependence methods FullDependence::FullDependence(Instruction *Source, Instruction *Destination, + const SCEVUnionPredicate &Assumes, bool PossiblyLoopIndependent, unsigned CommonLevels) - : Dependence(Source, Destination), Levels(CommonLevels), + : Dependence(Source, Destination, Assumes), Levels(CommonLevels), LoopIndependent(PossiblyLoopIndependent) { Consistent = true; if (CommonLevels) @@ -711,6 +712,12 @@ void Dependence::dump(raw_ostream &OS) const { OS << " splitable"; } OS << "!\n"; + + SCEVUnionPredicate Assumptions = getRuntimeAssumptions(); + if (!Assumptions.isAlwaysTrue()) { + OS << " Runtime Assumptions:\n"; + Assumptions.print(OS, 2); + } } // Returns NoAlias/MayAliass/MustAlias for two memory locations based upon their @@ -3574,7 +3581,7 @@ bool DependenceInfo::invalidate(Function &F, const PreservedAnalyses &PA, Inv.invalidate(F, PA); } -SCEVUnionPredicate DependenceInfo::getRuntimeAssumptions() { +SCEVUnionPredicate DependenceInfo::getRuntimeAssumptions() const { return SCEVUnionPredicate(Assumptions, *SE); } @@ -3590,7 +3597,9 @@ SCEVUnionPredicate DependenceInfo::getRuntimeAssumptions() { // Care is required to keep the routine below, getSplitIteration(), // up to date with respect to this routine. std::unique_ptr -DependenceInfo::depends(Instruction *Src, Instruction *Dst) { +DependenceInfo::depends(Instruction *Src, Instruction *Dst, + bool UnderRuntimeAssumptions) { + SmallVector Assume; bool PossiblyLoopIndependent = true; if (Src == Dst) PossiblyLoopIndependent = false; @@ -3602,7 +3611,8 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst) { if (!isLoadOrStore(Src) || !isLoadOrStore(Dst)) { // can only analyze simple loads and stores, i.e., no calls, invokes, etc. LLVM_DEBUG(dbgs() << "can only handle simple loads and stores\n"); - return std::make_unique(Src, Dst); + return std::make_unique(Src, Dst, + SCEVUnionPredicate(Assume, *SE)); } const MemoryLocation &DstLoc = MemoryLocation::get(Dst); @@ -3613,7 +3623,8 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst) { case AliasResult::PartialAlias: // cannot analyse objects if we don't understand their aliasing. LLVM_DEBUG(dbgs() << "can't analyze may or partial alias\n"); - return std::make_unique(Src, Dst); + return std::make_unique(Src, Dst, + SCEVUnionPredicate(Assume, *SE)); case AliasResult::NoAlias: // If the objects noalias, they are distinct, accesses are independent. LLVM_DEBUG(dbgs() << "no alias\n"); @@ -3626,7 +3637,8 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst) { // The dependence test gets confused if the size of the memory accesses // differ. LLVM_DEBUG(dbgs() << "can't analyze must alias with different sizes\n"); - return std::make_unique(Src, Dst); + return std::make_unique(Src, Dst, + SCEVUnionPredicate(Assume, *SE)); } Value *SrcPtr = getLoadStorePointerOperand(Src); @@ -3645,7 +3657,8 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst) { // We check this upfront so we don't crash in cases where getMinusSCEV() // returns a SCEVCouldNotCompute. LLVM_DEBUG(dbgs() << "can't analyze SCEV with different pointer base\n"); - return std::make_unique(Src, Dst); + return std::make_unique(Src, Dst, + SCEVUnionPredicate(Assume, *SE)); } uint64_t EltSize = SrcLoc.Size.toRaw(); @@ -3656,10 +3669,31 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst) { if (Src != Dst) { // Check that memory access offsets are multiples of element sizes. - if (!SE->isKnownMultipleOf(SrcEv, EltSize, Assumptions) || - !SE->isKnownMultipleOf(DstEv, EltSize, Assumptions)) { + if (!SE->isKnownMultipleOf(SrcEv, EltSize, Assume) || + !SE->isKnownMultipleOf(DstEv, EltSize, Assume)) { LLVM_DEBUG(dbgs() << "can't analyze SCEV with different offsets\n"); - return std::make_unique(Src, Dst); + return std::make_unique(Src, Dst, + SCEVUnionPredicate(Assume, *SE)); + } + } + + if (!Assume.empty()) { + if (!UnderRuntimeAssumptions) + return std::make_unique(Src, Dst, + SCEVUnionPredicate(Assume, *SE)); + if (Assumptions.empty()) { + Assumptions.append(Assume.begin(), Assume.end()); + } else { + // Add non-redundant assumptions. + unsigned N = Assumptions.size(); + for (const SCEVPredicate *P : Assume) { + bool Implied = false; + for (unsigned I = 0; I != N && !Implied; I++) + if (Assumptions[I]->implies(P, *SE)) + Implied = true; + if (!Implied) + Assumptions.push_back(P); + } } } @@ -3667,7 +3701,8 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst) { LLVM_DEBUG(dbgs() << " common nesting levels = " << CommonLevels << "\n"); LLVM_DEBUG(dbgs() << " maximum nesting levels = " << MaxLevels << "\n"); - FullDependence Result(Src, Dst, PossiblyLoopIndependent, CommonLevels); + FullDependence Result(Src, Dst, SCEVUnionPredicate(Assume, *SE), + PossiblyLoopIndependent, CommonLevels); ++TotalArrayPairs; unsigned Pairs = 1; @@ -4065,7 +4100,7 @@ const SCEV *DependenceInfo::getSplitIteration(const Dependence &Dep, // establish loop nesting levels establishNestingLevels(Src, Dst); - FullDependence Result(Src, Dst, false, CommonLevels); + FullDependence Result(Src, Dst, Dep.Assumptions, false, CommonLevels); unsigned Pairs = 1; SmallVector Pair(Pairs); diff --git a/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll b/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll index ba7ae7904cdbb..08e7b2627bcc1 100644 --- a/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll +++ b/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll @@ -30,6 +30,8 @@ define i32 @alias_with_parametric_offset(ptr nocapture %A, i64 %n) { ; CHECK-NEXT: da analyze - none! ; CHECK-NEXT: Src: store i32 2, ptr %arrayidx, align 1 --> Dst: %0 = load i32, ptr %A, align 1 ; CHECK-NEXT: da analyze - flow [|<]! +; CHECK-NEXT: Runtime Assumptions: +; CHECK-NEXT: Equal predicate: (zext i2 (trunc i64 %n to i2) to i64) == 0 ; CHECK-NEXT: Src: %0 = load i32, ptr %A, align 1 --> Dst: %0 = load i32, ptr %A, align 1 ; CHECK-NEXT: da analyze - none! ; CHECK-NEXT: Runtime Assumptions: @@ -48,6 +50,9 @@ define i32 @alias_with_parametric_expr(ptr nocapture %A, i64 %n, i64 %m) { ; CHECK-NEXT: da analyze - none! ; CHECK-NEXT: Src: store i32 2, ptr %arrayidx, align 1 --> Dst: %0 = load i32, ptr %arrayidx1, align 1 ; CHECK-NEXT: da analyze - flow [|<]! +; CHECK-NEXT: Runtime Assumptions: +; CHECK-NEXT: Equal predicate: (zext i2 ((trunc i64 %m to i2) + (-2 * (trunc i64 %n to i2))) to i64) == 0 +; CHECK-NEXT: Equal predicate: (zext i2 (-2 + (trunc i64 %m to i2)) to i64) == 0 ; CHECK-NEXT: Src: %0 = load i32, ptr %arrayidx1, align 1 --> Dst: %0 = load i32, ptr %arrayidx1, align 1 ; CHECK-NEXT: da analyze - none! ; CHECK-NEXT: Runtime Assumptions: diff --git a/llvm/test/Analysis/DependenceAnalysis/MIVCheckConst.ll b/llvm/test/Analysis/DependenceAnalysis/MIVCheckConst.ll index af814684b37cd..c1f8c85f2bf0e 100644 --- a/llvm/test/Analysis/DependenceAnalysis/MIVCheckConst.ll +++ b/llvm/test/Analysis/DependenceAnalysis/MIVCheckConst.ll @@ -42,6 +42,9 @@ define void @test(ptr %A, ptr %B, i1 %arg, i32 %n, i32 %m) #0 align 2 { ; CHECK-NEXT: da analyze - consistent input [0 S S]! ; CHECK-NEXT: Src: %v27 = load <32 x i32>, ptr %v25, align 256 --> Dst: %v32 = load <32 x i32>, ptr %v30, align 128 ; CHECK-NEXT: da analyze - input [* S S|<]! +; CHECK-NEXT: Runtime Assumptions: +; CHECK-NEXT: Equal predicate: (zext i7 (4 * (trunc i32 %v1 to i7) * (1 + (trunc i32 %n to i7))) to i32) == 0 +; CHECK-NEXT: Equal predicate: (8 * (zext i4 (trunc i32 %v1 to i4) to i32)) == 0 ; CHECK-NEXT: Src: %v32 = load <32 x i32>, ptr %v30, align 128 --> Dst: %v32 = load <32 x i32>, ptr %v30, align 128 ; CHECK-NEXT: da analyze - consistent input [0 S S]! ; CHECK-NEXT: Runtime Assumptions: From d9846e9ab86355de46d228d00340921ae2183140 Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Tue, 29 Apr 2025 14:05:52 +0000 Subject: [PATCH 17/24] add testcases --- .../DependenceAnalysis/DifferentOffsets.ll | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll b/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll index 08e7b2627bcc1..aae140059e310 100644 --- a/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll +++ b/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll @@ -92,3 +92,101 @@ entry: store i32 42, ptr %arrayidx1, align 4 ret i32 0 } + +define void @linearized_accesses(i64 %n, i64 %m, i64 %o, ptr %A) { +; CHECK-LABEL: 'linearized_accesses' +; CHECK-NEXT: Src: store i32 1, ptr %idx0, align 4 --> Dst: store i32 1, ptr %idx0, align 4 +; CHECK-NEXT: da analyze - output [* * *]! +; CHECK-NEXT: Src: store i32 1, ptr %idx0, align 4 --> Dst: store i32 1, ptr %idx1, align 4 +; CHECK-NEXT: da analyze - output [* * *|<]! +; CHECK-NEXT: Src: store i32 1, ptr %idx1, align 4 --> Dst: store i32 1, ptr %idx1, align 4 +; CHECK-NEXT: da analyze - none! +; +entry: + br label %for.i + +for.i: + %i = phi i64 [ 0, %entry ], [ %i.inc, %for.i.inc ] + br label %for.j + +for.j: + %j = phi i64 [ 0, %for.i ], [ %j.inc, %for.j.inc ] + br label %for.k + +for.k: + %k = phi i64 [ 0, %for.j ], [ %k.inc, %for.k.inc ] + %subscript0 = mul i64 %i, %m + %subscript1 = add i64 %j, %subscript0 + %subscript2 = mul i64 %subscript1, %o + %subscript3 = add i64 %subscript2, %k + %idx0 = getelementptr inbounds i64, ptr %A, i64 %subscript3 ; (i64*)(A) + i*m*o + j*o + k + store i32 1, ptr %idx0 + %idx1 = getelementptr inbounds i32, ptr %A, i64 %subscript3 ; (i32*)(A) + i*m*o + j*o + k + store i32 1, ptr %idx1 + br label %for.k.inc + +for.k.inc: + %k.inc = add nsw i64 %k, 1 + %k.exitcond = icmp eq i64 %k.inc, %o + br i1 %k.exitcond, label %for.j.inc, label %for.k + +for.j.inc: + %j.inc = add nsw i64 %j, 1 + %j.exitcond = icmp eq i64 %j.inc, %m + br i1 %j.exitcond, label %for.i.inc, label %for.j + +for.i.inc: + %i.inc = add nsw i64 %i, 1 + %i.exitcond = icmp eq i64 %i.inc, %n + br i1 %i.exitcond, label %end, label %for.i + +end: + ret void +} + +define void @multidim_accesses(ptr %A) { +; CHECK-LABEL: 'multidim_accesses' +; CHECK-NEXT: Src: store i32 1, ptr %idx0, align 4 --> Dst: store i32 1, ptr %idx0, align 4 +; CHECK-NEXT: da analyze - none! +; CHECK-NEXT: Src: store i32 1, ptr %idx0, align 4 --> Dst: store i32 1, ptr %idx1, align 4 +; CHECK-NEXT: da analyze - consistent output [0 0 0|<]! +; CHECK-NEXT: Src: store i32 1, ptr %idx1, align 4 --> Dst: store i32 1, ptr %idx1, align 4 +; CHECK-NEXT: da analyze - none! +; +entry: + br label %for.i + +for.i: + %i = phi i64 [ 0, %entry ], [ %i.inc, %for.i.inc ] + br label %for.j + +for.j: + %j = phi i64 [ 0, %for.i ], [ %j.inc, %for.j.inc ] + br label %for.k + +for.k: + %k = phi i64 [ 0, %for.j ], [ %k.inc, %for.k.inc ] + %idx0 = getelementptr inbounds [256 x [256 x [256 x i64]]], ptr %A, i64 %i, i64 %j, i64 %k + store i32 1, ptr %idx0 + %idx1 = getelementptr inbounds [256 x [256 x [256 x i32]]], ptr %A, i64 %i, i64 %j, i64 %k + store i32 1, ptr %idx1 + br label %for.k.inc + +for.k.inc: + %k.inc = add nsw i64 %k, 1 + %k.exitcond = icmp eq i64 %k.inc, 256 + br i1 %k.exitcond, label %for.j.inc, label %for.k + +for.j.inc: + %j.inc = add nsw i64 %j, 1 + %j.exitcond = icmp eq i64 %j.inc, 256 + br i1 %j.exitcond, label %for.i.inc, label %for.j + +for.i.inc: + %i.inc = add nsw i64 %i, 1 + %i.exitcond = icmp eq i64 %i.inc, 256 + br i1 %i.exitcond, label %end, label %for.i + +end: + ret void +} From b9542825190c6e2d97525670d7404b514a0a2eb3 Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Mon, 5 May 2025 14:06:37 +0000 Subject: [PATCH 18/24] Revert "disable loop interchange, fusion, and unroll-and-jam on runtime assumptions" This reverts commit 8c69ebf95b860081c084b542f107ccbc000afc8f. --- llvm/lib/Transforms/Scalar/LoopFuse.cpp | 18 +++--------------- llvm/lib/Transforms/Scalar/LoopInterchange.cpp | 12 +----------- llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp | 7 +------ 3 files changed, 5 insertions(+), 32 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/LoopFuse.cpp b/llvm/lib/Transforms/Scalar/LoopFuse.cpp index e91217302e5e5..5bba3016ba4a1 100644 --- a/llvm/lib/Transforms/Scalar/LoopFuse.cpp +++ b/llvm/lib/Transforms/Scalar/LoopFuse.cpp @@ -1129,11 +1129,7 @@ struct LoopFuser { } } } - SCEVUnionPredicate Assumptions = DI.getRuntimeAssumptions(); - // Fail if the dependence analysis has runtime assumptions. - // FIXME: do loop versioning to keep the original loop, and transform the - // loop under the runtime assumptions. - return Assumptions.isAlwaysTrue(); + return true; } // Returns true if the instruction \p I can be sunk to the top of the exit @@ -1176,11 +1172,7 @@ struct LoopFuser { } } - SCEVUnionPredicate Assumptions = DI.getRuntimeAssumptions(); - // Fail if the dependence analysis has runtime assumptions. - // FIXME: do loop versioning to keep the original loop, and transform the - // loop under the runtime assumptions. - return Assumptions.isAlwaysTrue(); + return true; } /// Collect instructions in the \p FC1 Preheader that can be hoisted @@ -1428,11 +1420,7 @@ struct LoopFuser { return false; } - SCEVUnionPredicate Assumptions = DI.getRuntimeAssumptions(); - // Fail if the dependence analysis has runtime assumptions. - // FIXME: do loop versioning to keep the original loop, and transform the - // loop under the runtime assumptions. - return Assumptions.isAlwaysTrue(); + return true; } /// Determine if two fusion candidates are adjacent in the CFG. diff --git a/llvm/lib/Transforms/Scalar/LoopInterchange.cpp b/llvm/lib/Transforms/Scalar/LoopInterchange.cpp index 0b618fcdc37c9..1dccba4cfa7b8 100644 --- a/llvm/lib/Transforms/Scalar/LoopInterchange.cpp +++ b/llvm/lib/Transforms/Scalar/LoopInterchange.cpp @@ -220,11 +220,7 @@ static bool populateDependencyMatrix(CharMatrix &DepMatrix, unsigned Level, } } - SCEVUnionPredicate Assumptions = DI->getRuntimeAssumptions(); - // Fail if the dependence analysis has runtime assumptions. - // FIXME: do loop versioning to keep the original loop, and transform the - // loop under the runtime assumptions. - return Assumptions.isAlwaysTrue(); + return true; } // A loop is moved from index 'from' to an index 'to'. Update the Dependence @@ -1838,12 +1834,6 @@ PreservedAnalyses LoopInterchangePass::run(LoopNest &LN, std::unique_ptr CC = CacheCost::getCacheCost(LN.getOutermostLoop(), AR, DI); - SCEVUnionPredicate Assumptions = DI.getRuntimeAssumptions(); - // Early fail when the dependence analysis has runtime assumptions. - // FIXME: this could be handled by versioning the loop. - if (!Assumptions.isAlwaysTrue()) - return PreservedAnalyses::all(); - if (!LoopInterchange(&AR.SE, &AR.LI, &DI, &AR.DT, CC, &ORE).run(LN)) return PreservedAnalyses::all(); U.markLoopNestChanged(true); diff --git a/llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp b/llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp index d501cb401cb4d..ca90bb65f5708 100644 --- a/llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp +++ b/llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp @@ -800,12 +800,7 @@ checkDependencies(Loop &Root, const BasicBlockSet &SubLoopBlocks, EarlierLoadsAndStores.append(CurrentLoadsAndStores.begin(), CurrentLoadsAndStores.end()); } - - SCEVUnionPredicate Assumptions = DI.getRuntimeAssumptions(); - // Fail if the dependence analysis has runtime assumptions. - // FIXME: do loop versioning to keep the original loop, and transform the - // loop under the runtime assumptions. - return Assumptions.isAlwaysTrue(); + return true; } static bool isEligibleLoopForm(const Loop &Root) { From 9a110dfd65f894c41453e1c0bae04e5642f2f124 Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Mon, 5 May 2025 19:31:51 +0000 Subject: [PATCH 19/24] turn off UnderRuntimeAssumptions in depends computation --- llvm/include/llvm/Analysis/DependenceAnalysis.h | 8 ++++---- llvm/lib/Analysis/DependenceAnalysis.cpp | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/llvm/include/llvm/Analysis/DependenceAnalysis.h b/llvm/include/llvm/Analysis/DependenceAnalysis.h index 553bdc191bc0c..6b715ab62331e 100644 --- a/llvm/include/llvm/Analysis/DependenceAnalysis.h +++ b/llvm/include/llvm/Analysis/DependenceAnalysis.h @@ -310,11 +310,11 @@ namespace llvm { /// Returns NULL if no dependence; otherwise, returns a Dependence (or a /// FullDependence) with as much information as can be gleaned. By default, /// the dependence test collects a set of runtime assumptions that cannot be - /// solved at compilation time. Set UnderRuntimeAssumptions to false for a - /// safe approximation of the dependence relation that does not require - /// runtime checks. + /// solved at compilation time. By default UnderRuntimeAssumptions is false + /// for a safe approximation of the dependence relation that does not + /// require runtime checks. std::unique_ptr depends(Instruction *Src, Instruction *Dst, - bool UnderRuntimeAssumptions = true); + bool UnderRuntimeAssumptions = false); /// getSplitIteration - Give a dependence that's splittable at some /// particular level, return the iteration that should be used to split diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp index 5d17ceb9dac2c..6134888ecc356 100644 --- a/llvm/lib/Analysis/DependenceAnalysis.cpp +++ b/llvm/lib/Analysis/DependenceAnalysis.cpp @@ -187,7 +187,7 @@ static void dumpExampleDependence(raw_ostream &OS, DependenceInfo *DA, if (DstI->mayReadOrWriteMemory()) { OS << "Src:" << *SrcI << " --> Dst:" << *DstI << "\n"; OS << " da analyze - "; - if (auto D = DA->depends(&*SrcI, &*DstI)) { + if (auto D = DA->depends(&*SrcI, &*DstI, true)) { // Normalize negative direction vectors if required by clients. if (NormalizeResults && D->normalize(&SE)) OS << "normalized - "; From 297ff4471a117ec8409ec5962614ab6a68429732 Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Tue, 6 May 2025 14:15:33 +0000 Subject: [PATCH 20/24] clang-format code around my changes --- llvm/lib/Analysis/DependenceAnalysis.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp index 6134888ecc356..10b5be72e82b3 100644 --- a/llvm/lib/Analysis/DependenceAnalysis.cpp +++ b/llvm/lib/Analysis/DependenceAnalysis.cpp @@ -199,8 +199,7 @@ static void dumpExampleDependence(raw_ostream &OS, DependenceInfo *DA, OS << "!\n"; } } - } - else + } else OS << "none!\n"; } } From 861ef01a54d9b0a04db115f31cf714321086dc35 Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Tue, 13 May 2025 02:08:09 +0000 Subject: [PATCH 21/24] update comments for last wave of reviews --- llvm/include/llvm/Analysis/ScalarEvolution.h | 6 ++++-- llvm/lib/Analysis/DependenceAnalysis.cpp | 5 ++--- llvm/lib/Analysis/ScalarEvolution.cpp | 6 +++++- .../test/Analysis/DependenceAnalysis/DifferentOffsets.ll | 9 +++++++++ 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/llvm/include/llvm/Analysis/ScalarEvolution.h b/llvm/include/llvm/Analysis/ScalarEvolution.h index eadc6752a323d..339bdfeb4956a 100644 --- a/llvm/include/llvm/Analysis/ScalarEvolution.h +++ b/llvm/include/llvm/Analysis/ScalarEvolution.h @@ -1044,8 +1044,10 @@ class ScalarEvolution { bool isKnownToBeAPowerOfTwo(const SCEV *S, bool OrZero = false, bool OrNegative = false); - /// Check that memory access offsets in S are multiples of M. Assumptions - /// records the runtime predicates under which S is a multiple of M. + /// Check that \p S is a multiple of \p M. When \p S is an AddRecExpr, \p S is + /// a multiple of \p M if \p S starts with a multiple of \p M and at every + /// iteration step \p S only adds multiples of \p M. \p Assumptions records + /// the runtime predicates under which \p S is a multiple of \p M. bool isKnownMultipleOf(const SCEV *S, uint64_t M, SmallVectorImpl &Assumptions); diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp index 10b5be72e82b3..652a1c4d0fae2 100644 --- a/llvm/lib/Analysis/DependenceAnalysis.cpp +++ b/llvm/lib/Analysis/DependenceAnalysis.cpp @@ -187,7 +187,8 @@ static void dumpExampleDependence(raw_ostream &OS, DependenceInfo *DA, if (DstI->mayReadOrWriteMemory()) { OS << "Src:" << *SrcI << " --> Dst:" << *DstI << "\n"; OS << " da analyze - "; - if (auto D = DA->depends(&*SrcI, &*DstI, true)) { + if (auto D = DA->depends(&*SrcI, &*DstI, + /*UnderRuntimeAssumptions=*/true)) { // Normalize negative direction vectors if required by clients. if (NormalizeResults && D->normalize(&SE)) OS << "normalized - "; @@ -3661,8 +3662,6 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst, } uint64_t EltSize = SrcLoc.Size.toRaw(); - assert(EltSize == DstLoc.Size.toRaw() && "Array element size differ"); - const SCEV *SrcEv = SE->getMinusSCEV(SrcSCEV, SrcBase); const SCEV *DstEv = SE->getMinusSCEV(DstSCEV, DstBase); diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp index ef80199b68c61..d94474a902519 100644 --- a/llvm/lib/Analysis/ScalarEvolution.cpp +++ b/llvm/lib/Analysis/ScalarEvolution.cpp @@ -10979,7 +10979,9 @@ bool ScalarEvolution::isKnownMultipleOf( if (M == 1) return true; - // Recursively check AddRec operands. + // Recursively check AddRec operands. An AddRecExpr S is a multiple of M if S + // starts with a multiple of M and at every iteration step S only adds + // multiples of M. if (auto *AddRec = dyn_cast(S)) return isKnownMultipleOf(AddRec->getStart(), M, Assumptions) && isKnownMultipleOf(AddRec->getStepRecurrence(*this), M, Assumptions); @@ -10990,6 +10992,8 @@ bool ScalarEvolution::isKnownMultipleOf( return C.urem(M) == 0; } + // TODO: Also check other SCEV expressions, i.e., SCEVAddRecExpr, etc. + // Basic tests have failed. // Check "S % M == 0" at compile time and record runtime Assumptions. auto *STy = dyn_cast(S->getType()); diff --git a/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll b/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll index aae140059e310..1f8fac3087bff 100644 --- a/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll +++ b/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll @@ -149,10 +149,19 @@ define void @multidim_accesses(ptr %A) { ; CHECK-NEXT: Src: store i32 1, ptr %idx0, align 4 --> Dst: store i32 1, ptr %idx0, align 4 ; CHECK-NEXT: da analyze - none! ; CHECK-NEXT: Src: store i32 1, ptr %idx0, align 4 --> Dst: store i32 1, ptr %idx1, align 4 +; FIXME: the dependence distance is not constant. Distance vector should be [* * *|<]! ; CHECK-NEXT: da analyze - consistent output [0 0 0|<]! ; CHECK-NEXT: Src: store i32 1, ptr %idx1, align 4 --> Dst: store i32 1, ptr %idx1, align 4 ; CHECK-NEXT: da analyze - none! ; +; for (i = 0; i < 256; i++) +; for (j = 0; j < 256; j++) +; for (k = 0; k < 256; k++) { +; int *idx0 = (int *)((long long *)(A) + 256*256*i + 256*j + k); +; *idx0 = 1; +; int *idx1 = (int *)((int *)(A) + 256*256*i + 256*j + k); +; *idx1 = 1; +; } entry: br label %for.i From 4972776ba790ce1e598e6fef745d5e11912584c0 Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Tue, 13 May 2025 03:17:45 +0000 Subject: [PATCH 22/24] also reject before or after pointer --- llvm/lib/Analysis/DependenceAnalysis.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp index 652a1c4d0fae2..d1cc8281b8877 100644 --- a/llvm/lib/Analysis/DependenceAnalysis.cpp +++ b/llvm/lib/Analysis/DependenceAnalysis.cpp @@ -3633,7 +3633,8 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst, break; // The underlying objects alias; test accesses for dependence. } - if (DstLoc.Size != SrcLoc.Size) { + if (DstLoc.Size != SrcLoc.Size || DstLoc.Size.mayBeBeforePointer() || + SrcLoc.Size.mayBeBeforePointer()) { // The dependence test gets confused if the size of the memory accesses // differ. LLVM_DEBUG(dbgs() << "can't analyze must alias with different sizes\n"); From 338668f5250da875d3b34a29602219d595386523 Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Tue, 13 May 2025 08:38:51 -0500 Subject: [PATCH 23/24] use LocationSize.isPrecise Co-authored-by: Ryotaro Kasuga --- llvm/lib/Analysis/DependenceAnalysis.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp index d1cc8281b8877..3278f594dd84b 100644 --- a/llvm/lib/Analysis/DependenceAnalysis.cpp +++ b/llvm/lib/Analysis/DependenceAnalysis.cpp @@ -3633,8 +3633,8 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst, break; // The underlying objects alias; test accesses for dependence. } - if (DstLoc.Size != SrcLoc.Size || DstLoc.Size.mayBeBeforePointer() || - SrcLoc.Size.mayBeBeforePointer()) { + if (DstLoc.Size != SrcLoc.Size || !DstLoc.Size.isPrecise() || + !SrcLoc.Size.isPrecise()) { // The dependence test gets confused if the size of the memory accesses // differ. LLVM_DEBUG(dbgs() << "can't analyze must alias with different sizes\n"); From 869b5ff5804671fa934ce288e2f6cc2b96136a8a Mon Sep 17 00:00:00 2001 From: Sebastian Pop Date: Tue, 13 May 2025 13:45:38 +0000 Subject: [PATCH 24/24] remove Assumptions.empty branch --- llvm/lib/Analysis/DependenceAnalysis.cpp | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp index 3278f594dd84b..dba3ac28b37a3 100644 --- a/llvm/lib/Analysis/DependenceAnalysis.cpp +++ b/llvm/lib/Analysis/DependenceAnalysis.cpp @@ -3680,19 +3680,15 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst, if (!UnderRuntimeAssumptions) return std::make_unique(Src, Dst, SCEVUnionPredicate(Assume, *SE)); - if (Assumptions.empty()) { - Assumptions.append(Assume.begin(), Assume.end()); - } else { - // Add non-redundant assumptions. - unsigned N = Assumptions.size(); - for (const SCEVPredicate *P : Assume) { - bool Implied = false; - for (unsigned I = 0; I != N && !Implied; I++) - if (Assumptions[I]->implies(P, *SE)) - Implied = true; - if (!Implied) - Assumptions.push_back(P); - } + // Add non-redundant assumptions. + unsigned N = Assumptions.size(); + for (const SCEVPredicate *P : Assume) { + bool Implied = false; + for (unsigned I = 0; I != N && !Implied; I++) + if (Assumptions[I]->implies(P, *SE)) + Implied = true; + if (!Implied) + Assumptions.push_back(P); } }