[Transforms][Utils] Add LoopSplitUtils for iteration-space loop splitting - #205995
Conversation
|
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-llvm-transforms Author: Ashutosh Nema (nema-ashutosh) ChangesIntroduce LoopSplitUtils, a utility that splits a counted loop into a chain of per-partition sub-loops covering contiguous slices of the original iteration space. Given a loop and a list of partition ranges, it clones the body per partition, guards each with an entry check that skips empty partitions, clamps each latch to its slice, and rebuilds SSA for loop-carried and live-out values so the result is behaviour-preserving. Key properties:
How to use: LoopSplitUtils LSU(L, LI, SE, DT); A new test pass, loop-split-test (opt -passes=loop-split-test with -loop-split-points=...), drives the utility for testing. Adds lit tests covering basic/multiple/four-partition splits, descending loops, reductions, empty-leading partitions, optional guards, and the per-partition value map. Patch is 67.08 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/205995.diff 15 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Utils/LoopSplitTestPass.h b/llvm/include/llvm/Transforms/Utils/LoopSplitTestPass.h
new file mode 100644
index 0000000000000..1e427f02a542f
--- /dev/null
+++ b/llvm/include/llvm/Transforms/Utils/LoopSplitTestPass.h
@@ -0,0 +1,29 @@
+//===- LoopSplitTestPass.h - Test driver for LoopSplitUtils -----*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// A command-line driven pass used to exercise the LoopSplitUtils utility from
+// `opt`. The split points are provided via the -loop-split-points option as
+// iteration offsets relative to the induction start.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TRANSFORMS_UTILS_LOOPSPLITTESTPASS_H
+#define LLVM_TRANSFORMS_UTILS_LOOPSPLITTESTPASS_H
+
+#include "llvm/IR/PassManager.h"
+
+namespace llvm {
+
+class LoopSplitTestPass : public PassInfoMixin<LoopSplitTestPass> {
+public:
+ PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
+};
+
+} // namespace llvm
+
+#endif // LLVM_TRANSFORMS_UTILS_LOOPSPLITTESTPASS_H
diff --git a/llvm/include/llvm/Transforms/Utils/LoopSplitUtils.h b/llvm/include/llvm/Transforms/Utils/LoopSplitUtils.h
new file mode 100644
index 0000000000000..1d68e8db5d774
--- /dev/null
+++ b/llvm/include/llvm/Transforms/Utils/LoopSplitUtils.h
@@ -0,0 +1,163 @@
+//===- LoopSplitUtils.h - Split a loop's iteration space --------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Splits a counted loop's iteration space into a chain of per-partition
+// sub-loops. See LoopSplitUtils.cpp for the structure produced.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TRANSFORMS_UTILS_LOOPSPLITUTILS_H
+#define LLVM_TRANSFORMS_UTILS_LOOPSPLITUTILS_H
+
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/Compiler.h"
+#include "llvm/Transforms/Utils/ValueMapper.h"
+#include <memory>
+
+namespace llvm {
+
+class BasicBlock;
+class DominatorTree;
+class ICmpInst;
+class Instruction;
+class Loop;
+class LoopInfo;
+class PHINode;
+class SCEV;
+class SCEVAddRecExpr;
+class ScalarEvolution;
+class Value;
+
+/// Splits a counted loop into a chain of per-partition sub-loops.
+///
+/// Usage:
+/// \code
+/// LoopSplitUtils LSU(L, LI, SE, DT);
+/// if (!LSU.isLegal())
+/// return false;
+/// LSU.addPartition(S0, E0); // one call per partition, in order
+/// LSU.addPartition(S1, E1);
+/// LSU.split();
+/// \endcode
+class LoopSplitUtils {
+public:
+ LoopSplitUtils(Loop *L, LoopInfo *LI, ScalarEvolution *SE, DominatorTree *DT)
+ : L(L), LI(LI), SE(SE), DT(DT) {}
+
+ /// Analyze \p L and return true if it is a counted loop this utility can split:
+ /// a bottom-tested single-exit loop in LCSSA form with a unique unit-step
+ /// integer induction and a computable trip count. Must succeed before split().
+ LLVM_ABI bool isLegal();
+
+ /// Return the loop's induction variable. Valid only after isLegal() succeeds.
+ PHINode *getInductionVariable() const { return Induction; }
+
+ /// Append an inclusive partition range [Start, End] in iteration order.
+ /// Partitions must tile the whole space: first Start = induction start, each
+ /// later Start = previous End +/- step, last End = induction end (desc: S >= E).
+ ///
+ /// Bounds must be loop-invariant and representable in the induction type
+ /// without wrapping: a Start +/- offset that wraps past TYPE_MAX/MIN/0 looks
+ /// in-range and silently miscompiles. See LoopSplitUtils.cpp for the rationale.
+ ///
+ /// Every partition is guarded by default; use avoidPartitionGuard() to opt out.
+ LLVM_ABI void addPartition(const SCEV *Start, const SCEV *End);
+
+ /// Suppress the entry guard for partition \p PartitionIndex (already added). Use
+ /// only for a partition the caller can prove runs at least once; for a runtime-
+ /// empty partition this is incorrect and yields one spurious iteration.
+ LLVM_ABI void avoidPartitionGuard(unsigned PartitionIndex);
+
+ unsigned getNumPartitions() const { return Partitions.size(); }
+
+ /// Perform the split. Requires a successful isLegal() and at least two
+ /// partitions. Returns true if the loop was rewritten.
+ LLVM_ABI bool split();
+
+ /// Return the counterpart of original-loop value \p V in partition
+ /// \p PartitionIndex (0-based). Partition 0 maps values to themselves; a later
+ /// partition returns the clone, or null if not cloned. Valid only after split().
+ LLVM_ABI Value *getPartitionValue(const Value *V,
+ unsigned PartitionIndex) const;
+
+ /// Return the original-to-clone value map for the partition at
+ /// \p PartitionIndex, for callers that want to remap many values. Null for
+ /// partition 0 (identity) and for any partition that was not cloned.
+ LLVM_ABI const ValueToValueMapTy *
+ getPartitionValueMap(unsigned PartitionIndex) const;
+
+private:
+ /// Everything known about one partition: the caller-supplied range plus the
+ /// state split() derives. Indexed by partition number in \c Partitions.
+ struct PartitionInfo {
+ // Set by addPartition() / avoidPartitionGuard() before split():
+ const SCEV *StartExpr = nullptr; // inclusive iteration range [Start, End].
+ const SCEV *EndExpr = nullptr;
+ bool Guarded = true; // emit an entry guard?
+
+ // Filled in by split():
+ std::unique_ptr<ValueToValueMapTy> VMap; // null for partition 0 (identity).
+ Value *StartVal = nullptr; // expanded start.
+ Value *SelEnd = nullptr; // clamped end min(End, indEnd).
+ bool Empty = false; // provably zero-iteration.
+ BasicBlock *GuardBlock = nullptr;
+ BasicBlock *Preheader = nullptr;
+ BasicBlock *Exit = nullptr;
+ Loop *SubLoop = nullptr;
+ Value *LatchIndOp = nullptr; // induction operand of the latch compare.
+ };
+
+ /// Per-split() scratch threaded through the phase helpers (the escaping
+ /// values, new blocks, etc.). A pure transform internal, so it is defined in
+ /// the implementation file.
+ struct SplitState;
+
+ Loop *L;
+ LoopInfo *LI;
+ ScalarEvolution *SE;
+ DominatorTree *DT;
+
+ // Induction analysis, populated by isLegal().
+ PHINode *Induction = nullptr;
+ ICmpInst *LatchCmp = nullptr; // the loop's latch exit compare.
+ Value *LatchIndOperand = nullptr; // induction operand of the latch compare.
+ bool LatchUsesInductionPHI = false; // latch compares the PHI, not the step.
+ bool InductionIsSigned = false; // iteration ordering signedness.
+ bool InductionIsDescending = false; // step is -1 (loop counts down).
+ const SCEV *InductionEnd = nullptr;
+
+ /// One record per partition, in add order.
+ SmallVector<PartitionInfo, 4> Partitions;
+
+ /// Find and validate the induction recurrence; returns its add-recurrence, or
+ /// null if the loop has no suitable induction.
+ const SCEVAddRecExpr *analyzeInduction();
+ /// Determine the signedness of the iteration ordering from the latch compare
+ /// and the recurrence's no-wrap flags; returns false if it cannot be proven.
+ bool computeSignedness(const SCEVAddRecExpr *IndAR);
+
+ // split() phase helpers, run in order; each is documented at its definition.
+ /// Collect loop-carried and live-out values and split off the final exit.
+ void collectEscapingValues(SplitState &S);
+ /// Insert the entry guard ahead of partition 0 and update the dominator tree.
+ void buildEntryGuard(SplitState &S);
+ /// Expand each partition's start and clamped end into the entry guard.
+ void expandPartitionBounds(SplitState &S);
+ /// Pass 1: clone each later partition's sub-loop and create its guard/exit.
+ void clonePartitions(SplitState &S);
+ /// Pass 2: emit each guard, clamp each latch, and chain the partitions.
+ void chainPartitions(SplitState &S);
+ /// Rebuild SSA for every escaping value with a per-value SSAUpdater.
+ void reconstructSSA(SplitState &S);
+ /// Clamp \p PL's latch so it iterates only within [start, \p SelEnd].
+ void rewriteLatch(Loop *PL, Value *IndOp, Value *SelEnd, BasicBlock *Exit);
+};
+
+} // namespace llvm
+
+#endif // LLVM_TRANSFORMS_UTILS_LOOPSPLITUTILS_H
diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index 603d7f2f5dea2..4674e4c3c5bd3 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -368,6 +368,7 @@
#include "llvm/Transforms/Utils/InstructionNamer.h"
#include "llvm/Transforms/Utils/LibCallsShrinkWrap.h"
#include "llvm/Transforms/Utils/LoopSimplify.h"
+#include "llvm/Transforms/Utils/LoopSplitTestPass.h"
#include "llvm/Transforms/Utils/LoopVersioning.h"
#include "llvm/Transforms/Utils/LowerGlobalDtors.h"
#include "llvm/Transforms/Utils/LowerIFunc.h"
diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
index 9edb30fedd867..7970434b91dfe 100644
--- a/llvm/lib/Passes/PassRegistry.def
+++ b/llvm/lib/Passes/PassRegistry.def
@@ -481,6 +481,7 @@ FUNCTION_PASS("loop-fusion", LoopFusePass())
FUNCTION_PASS("loop-load-elim", LoopLoadEliminationPass())
FUNCTION_PASS("loop-simplify", LoopSimplifyPass())
FUNCTION_PASS("loop-sink", LoopSinkPass())
+FUNCTION_PASS("loop-split-test", LoopSplitTestPass())
FUNCTION_PASS("loop-versioning", LoopVersioningPass())
FUNCTION_PASS("lower-atomic", LowerAtomicPass())
FUNCTION_PASS("lower-constant-intrinsics", LowerConstantIntrinsicsPass())
diff --git a/llvm/lib/Transforms/Utils/CMakeLists.txt b/llvm/lib/Transforms/Utils/CMakeLists.txt
index 933e204081ad2..6163a3019e487 100644
--- a/llvm/lib/Transforms/Utils/CMakeLists.txt
+++ b/llvm/lib/Transforms/Utils/CMakeLists.txt
@@ -48,6 +48,8 @@ add_llvm_component_library(LLVMTransformUtils
LoopPeel.cpp
LoopRotationUtils.cpp
LoopSimplify.cpp
+ LoopSplitTestPass.cpp
+ LoopSplitUtils.cpp
LoopUnroll.cpp
LoopUnrollAndJam.cpp
LoopUnrollRuntime.cpp
diff --git a/llvm/lib/Transforms/Utils/LoopSplitTestPass.cpp b/llvm/lib/Transforms/Utils/LoopSplitTestPass.cpp
new file mode 100644
index 0000000000000..b94ab1ee82496
--- /dev/null
+++ b/llvm/lib/Transforms/Utils/LoopSplitTestPass.cpp
@@ -0,0 +1,151 @@
+//===- LoopSplitTestPass.cpp - Test driver for LoopSplitUtils -------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This pass drives LoopSplitUtils from `opt` for testing. For every eligible
+// loop it builds partitions from the -loop-split-points offsets and splits the
+// loop.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Transforms/Utils/LoopSplitTestPass.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/Analysis/LoopInfo.h"
+#include "llvm/Analysis/ScalarEvolution.h"
+#include "llvm/Analysis/ScalarEvolutionExpressions.h"
+#include "llvm/IR/Dominators.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/ValueHandle.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Transforms/Utils/LoopSplitUtils.h"
+
+using namespace llvm;
+
+#define DEBUG_TYPE "loop-split-test"
+
+static cl::list<unsigned>
+ SplitPoints("loop-split-points",
+ cl::desc("Iteration offsets (relative to the induction start) "
+ "at which to split each loop"),
+ cl::CommaSeparated);
+
+static cl::list<unsigned> UnguardedPartitions(
+ "loop-split-unguarded",
+ cl::desc("Partition indices whose entry guard is omitted (the caller "
+ "guarantees they run at least one iteration)"),
+ cl::CommaSeparated);
+
+static cl::opt<bool> PrintPartitionMap(
+ "loop-split-print-partition-map",
+ cl::desc("After splitting, print each original loop instruction's "
+ "counterpart in every partition (LoopSplitUtils::getPartitionValue)"),
+ cl::init(false));
+
+/// Build the partition list for \p L from the command-line split offsets and
+/// run the transform. Returns true if the loop was split.
+static bool splitLoop(Loop *L, ScalarEvolution &SE, DominatorTree &DT,
+ LoopInfo &LI) {
+ LoopSplitUtils LSU(L, &LI, &SE, &DT);
+ if (!LSU.isLegal()) {
+ LLVM_DEBUG(dbgs() << "loop-split-test: loop is not legal for splitting\n");
+ return false;
+ }
+
+ const auto *IndAR =
+ dyn_cast<SCEVAddRecExpr>(SE.getSCEV(LSU.getInductionVariable()));
+ if (!IndAR)
+ return false;
+
+ const SCEV *Start = IndAR->getStart();
+ const SCEV *BTC = SE.getBackedgeTakenCount(L);
+ const SCEV *End = IndAR->evaluateAtIteration(BTC, SE);
+ Type *Ty = Start->getType();
+ if (End->getType() != Ty)
+ End = SE.getTruncateExpr(End, Ty);
+
+ // Build boundaries in iteration order, stepping away from Start by each
+ // offset (down for a descending loop). Each offset opens a new partition at
+ // iteration `Start +/- offset`; the previous partition ends one step before.
+ bool Descending = false;
+ if (const auto *StepC = dyn_cast<SCEVConstant>(IndAR->getStepRecurrence(SE)))
+ Descending = StepC->getValue()->isMinusOne();
+
+ const SCEV *PrevStart = Start;
+ const SCEV *One = SE.getOne(Ty);
+ for (unsigned Offset : SplitPoints) {
+ const SCEV *Off = SE.getConstant(Ty, Offset);
+ const SCEV *Point =
+ Descending ? SE.getMinusSCEV(Start, Off) : SE.getAddExpr(Start, Off);
+ const SCEV *PrevEnd =
+ Descending ? SE.getAddExpr(Point, One) : SE.getMinusSCEV(Point, One);
+ LSU.addPartition(PrevStart, PrevEnd);
+ PrevStart = Point;
+ }
+ // The final partition runs to the iteration-space end.
+ LSU.addPartition(PrevStart, End);
+
+ // Suppress guards for the partitions the caller listed (out-of-range indices
+ // are ignored).
+ for (unsigned Idx : UnguardedPartitions)
+ if (Idx < LSU.getNumPartitions())
+ LSU.avoidPartitionGuard(Idx);
+
+ if (LSU.getNumPartitions() < 2)
+ return false;
+
+ // Snapshot the original loop's named instructions before the transform so we
+ // can query their per-partition counterparts afterwards (handles track any
+ // that the transform deletes).
+ SmallVector<WeakTrackingVH, 16> OrigValues;
+ if (PrintPartitionMap)
+ for (BasicBlock *BB : L->blocks())
+ for (Instruction &I : *BB)
+ if (I.hasName())
+ OrigValues.push_back(&I);
+
+ if (!LSU.split())
+ return false;
+
+ if (PrintPartitionMap) {
+ const unsigned N = LSU.getNumPartitions();
+ for (unsigned P = 0; P < N; ++P) {
+ outs() << "LS-MAP partition " << P << ":\n";
+ for (WeakTrackingVH &VH : OrigValues) {
+ if (!VH)
+ continue;
+ Value *M = LSU.getPartitionValue(VH, P);
+ outs() << "LS-MAP " << VH->getName() << " -> "
+ << (M ? M->getName() : "<none>") << "\n";
+ }
+ }
+ }
+ return true;
+}
+
+PreservedAnalyses LoopSplitTestPass::run(Function &F,
+ FunctionAnalysisManager &AM) {
+ if (SplitPoints.empty())
+ return PreservedAnalyses::all();
+
+ auto &LI = AM.getResult<LoopAnalysis>(F);
+ auto &SE = AM.getResult<ScalarEvolutionAnalysis>(F);
+ auto &DT = AM.getResult<DominatorTreeAnalysis>(F);
+
+ // Collect the original top-level loops up front; the transform creates new
+ // sub-loops that we must not revisit.
+ SmallVector<Loop *, 4> Worklist(LI.begin(), LI.end());
+
+ bool Changed = false;
+ for (Loop *L : Worklist) {
+ SE.forgetLoop(L);
+ Changed |= splitLoop(L, SE, DT, LI);
+ }
+
+ return Changed ? PreservedAnalyses::none() : PreservedAnalyses::all();
+}
diff --git a/llvm/lib/Transforms/Utils/LoopSplitUtils.cpp b/llvm/lib/Transforms/Utils/LoopSplitUtils.cpp
new file mode 100644
index 0000000000000..54a3d616f902f
--- /dev/null
+++ b/llvm/lib/Transforms/Utils/LoopSplitUtils.cpp
@@ -0,0 +1,595 @@
+//===- LoopSplitUtils.cpp - Split a loop's iteration space ----------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Splits a counted loop's iteration space into a chain of per-partition
+// sub-loops. See LoopSplitUtils.h for the high-level usage guidelines.
+//
+// Structure produced for partitions [S0,E0], [S1,E1], ... where E is the loop's
+// last iteration and each clamped end sel_i = min(E_i, E):
+//
+// guard0: ; every S_i and sel_i is computed here
+// if (S0 <= sel0) goto preheader0 else goto guard1 ; default guard check
+// loop0: ... ; latch stops at sel0
+// exit0 -> guard1
+// guard1:
+// if (S1 <= sel1) goto preheader1 else goto guard2 ; default guard check
+// loop1: ... ; latch stops at sel1
+// exit1 -> guard2
+// ...
+// final.exit: ; merges every partition's live-outs
+//
+// Each guard holds the "S_i <= sel_i" check and skips an empty partition by
+// falling through to the next guard. The check is replaced by an unconditional
+// branch when a partition is proven empty (to the next guard) or the caller
+// exempts it via avoidPartitionGuard() (to its preheader). All S_i/sel_i are
+// materialized once in guard0; the end clamp keeps the "runs at least once"
+// iteration in the right partition; live-outs are rebuilt one SSAUpdater each.
+//
+// A descending (step -1) loop uses the same structure mirrored: partitions run
+// high-to-low and the empty test, clamp, and predicates flip (>=/>).
+//
+// Usage guidelines:
+// - Caller bounds must not wrap the induction type. The clamp absorbs a bound
+// past the runtime trip count, but a Start +/- offset that overshoots the
+// type extreme wraps in the bound arithmetic and cannot be repaired here.
+// - Bounds must be loop-invariant: they are expanded in guard0 (the preheader),
+// so a bound depending on a value defined inside the loop cannot be placed.
+// - The partitions must tile the original iteration space exactly -- same
+// iterations, same order -- so the split preserves program behaviour.
+// - A caller that drops a guard via avoidPartitionGuard() must itself ensure
+// that partition runs at least once, or the result is a spurious iteration.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Transforms/Utils/LoopSplitUtils.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/Analysis/LoopInfo.h"
+#include "llvm/Analysis/ScalarEvolution.h"
+#include "llvm/Analysis/ScalarEvolutionExpressions.h"
+#include "llvm/IR/BasicBlock.h"
+#include "llvm/IR/CFG.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/IR/Dominators.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/Instructions.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Transforms/Utils/BasicBlockUtils.h"
+#include "llvm/Transforms/Utils/Cloning.h"
+#include "llvm/Transforms/Utils/LoopUtils.h"
+#include "llvm/Transforms/Utils/SSAUpdater.h"
+#include "llvm/Transforms/Utils/ScalarEvolutionExpander.h"
+#include "llvm/Transforms/Utils/ValueMapper.h"
+
+using namespace llvm;
+
+#define DEBUG_TYPE "loop-split-utils"
+
+//===----------------------------------------------------------------------===//
+// LoopSplitUtils - construction, partition list, induction analysis
+//===----------------------------------------------------------------------===//
+
+/// Per-split() scratch shared by the phase helpers; lives for one split() call.
+struct LoopSplitUtils::SplitState {
+ BasicBlock *OrigPreheader = nullptr; // also pa...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
You can test this locally with the following command:Build idt from compnerd/ids, then for each changed header:
idt -p build/ --main-file <matching-source.cpp> \
--apply-fixits --inplace <header>View the diff from ids-check here.diff --git a/llvm/include/llvm/Transforms/Utils/LoopSplitTestPass.h b/llvm/include/llvm/Transforms/Utils/LoopSplitTestPass.h
index 1e427f02a..f8f1a7e4a 100644
--- a/llvm/include/llvm/Transforms/Utils/LoopSplitTestPass.h
+++ b/llvm/include/llvm/Transforms/Utils/LoopSplitTestPass.h
@@ -21,7 +21,7 @@ namespace llvm {
class LoopSplitTestPass : public PassInfoMixin<LoopSplitTestPass> {
public:
- PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
+ LLVM_ABI PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
};
} // namespace llvm
|
|
Migration plan with initial patch is available at #209142 kindly check |
|
@nikic - can u help by adding the reviewers here, so the review can be initiated ! |
nema-ashutosh
left a comment
There was a problem hiding this comment.
Thank you for your review and comments. I'm working through the feedback and will post an updated version of the PR soon with the suggested changes addressed.
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
artagnon
left a comment
There was a problem hiding this comment.
Could fix the build as well?
artagnon
left a comment
There was a problem hiding this comment.
LGTM, thanks for all the work! I think it's good enough for the tree now, and any improvements can be made in-tree.
|
@artagnon - can u help me to push this change, as i don't have commit access ? |
|
Could you kindly rebase the branch or merge main, just to make sure that nothing has changed, before we land this? |
…ting
Introduce LoopSplitUtils, a utility that splits a counted loop into a
chain of per-partition sub-loops covering contiguous slices of the
original iteration space. Given a loop and a list of partition ranges,
it clones the body per partition, guards each with an entry check that
skips empty partitions, clamps each latch to its slice, and rebuilds SSA
for loop-carried and live-out values so the result is behaviour-preserving.
Key properties:
- Supports ascending (+1) and descending (-1) unit-step inductions, in
both signed and unsigned iteration orderings, with direction-aware
guard/latch predicates and end clamps.
- Reuses the original loop for partition 0 and clones the rest, exposing
per-partition value maps via getPartitionValue()/getPartitionValueMap().
- Lets callers drop the entry guard for a partition proven non-empty via
avoidPartitionGuard(); provably-empty partitions are always skipped.
- Patches the dominator tree and LoopInfo incrementally rather than
rebuilding them.
How to use:
LoopSplitUtils LSU(L, LI, SE, DT);
if (!LSU.isLegal()) // counted, bottom-tested, LCSSA, unit step
return false;
// Tile the iteration space in order; e.g. split [Start, End] at K:
LSU.addPartition(Start, K - 1); // partition 0: [Start, K-1]
LSU.addPartition(K, End); // partition 1: [K, End]
LSU.split();
// After split(), query a cloned value in a given partition:
Value *V1 = LSU.getPartitionValue(Orig, /*PartitionIndex=*/1);
A new test pass, loop-split-test (opt -passes=loop-split-test with
-loop-split-points=...), drives the utility for testing. Adds lit tests
covering basic/multiple/four-partition splits, descending loops,
reductions, empty-leading partitions, optional guards, and the
per-partition value map.
- Drop cached members duplicating Loop/SCEV or other state: Induction, LatchCmp, InductionIsDescending, LatchUsesInductionPHI (recomputed or threaded via SplitState). - Make analyzeInduction, computeSignedness, buildEntryGuard, and rewriteLatch file-static helpers. - Simplify getPartitionValue/remapValue to lookup()/lookup_or(); add ValueMap::lookup_or. - Use make_early_inc_range in reconstructSSA; hoist IRBuilder in chainPartitions; add LLVM_ABI to LoopSplitTestPass::run.
Simplify EscapingValue/addPartition construction, drop a const_cast in getPartitionValue, inline getInductionVariable into the header (trimming redundant forward declarations), adopt m_scev_AffineAddRec, inline remapValue, guard SCEV expansion with SCEVExpanderCleaner, and use UncondBrInst::Create for placeholder branches.
Fix the -Wmissing-field-initializers build, adopt m_scev_APInt, use takeName for the preheader name, move the test-pass map dump to LLVM_DEBUG, and drop pre-passes from the test RUN lines.
Rename the test pass to LoopSplitUtilsPass, simplify legality checks, add tests.
f44c9da to
ef53cda
Compare
|
Thanks @artagnon for the detailed review! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/223/builds/8524 Here is the relevant piece of the build log for the reference |
This is a real failure; I'm afraid we have to revert and re-land unless there is a quick fix? |
|
Thanks, i have noticed that. it's the guard/latch branches not propagating profile metadata. I have a fix and will send a quick follow up PR shortly., sorry for the breakage ! |
No worries, we will wait for your follow-up PR :) |
|
#213626 - profile metadata issue , fix available |
…ting (llvm#205995) Introduce LoopSplitUtils, a utility that splits a counted loop into a chain of per-partition sub-loops covering contiguous slices of the original iteration space. Given a loop and a list of partition ranges, it clones the body per partition, guards each with an entry check that skips empty partitions, clamps each latch to its slice, and rebuilds SSA for loop-carried and live-out values so the result is behaviour-preserving. Key properties: - Supports ascending (+1) and descending (-1) unit-step inductions, in both signed and unsigned iteration orderings, with direction-aware guard/latch predicates and end clamps. - Reuses the original loop for partition 0 and clones the rest, exposing per-partition value maps via getPartitionValue()/getPartitionValueMap(). - Lets callers drop the entry guard for a partition proven non-empty via avoidPartitionGuard(); provably-empty partitions are always skipped. - Patches the dominator tree and LoopInfo incrementally rather than rebuilding them. How to use: LoopSplitUtils LSU(L, LI, SE, DT); if (!LSU.isLegal()) // counted, bottom-tested, LCSSA, unit step return false; // Tile the iteration space in order; e.g. split [Start, End] at K: LSU.addPartition(Start, K - 1); // partition 0: [Start, K-1] LSU.addPartition(K, End); // partition 1: [K, End] LSU.split(); // After split(), query a cloned value in a given partition: Value *V1 = LSU.getPartitionValue(Orig, /*PartitionIndex=*/1); A new test pass, loop-split-test (opt -passes=loop-split-test with -loop-split-points=...), drives the utility for testing. Adds lit tests covering basic/multiple/four-partition splits, descending loops, reductions, empty-leading partitions, optional guards, and the per-partition value map.
| // | ||
| //===----------------------------------------------------------------------===// | ||
| // | ||
| // Splits a counted loop's iteration space into a chain of per-partition |
There was a problem hiding this comment.
drive-by comment: LoopSplitUtilsPass, as a name, says very little. How about LoopPartitioningPass?
See https://llvm.org/docs/CodingStandards.html#the-low-level-issues
"Utils", (or "Helper", while at it) aren't precise, especially for a pass name. (Plenty of articles online about these suffixes as a naming antipattern)
There was a problem hiding this comment.
The pass is not meant to be a standalone pass -- LoopSplitUtils exposes an API, a bit like LoopInfo, and it's meant to be used via the API. The pass is purely for testing.
…ting (llvm#205995) Introduce LoopSplitUtils, a utility that splits a counted loop into a chain of per-partition sub-loops covering contiguous slices of the original iteration space. Given a loop and a list of partition ranges, it clones the body per partition, guards each with an entry check that skips empty partitions, clamps each latch to its slice, and rebuilds SSA for loop-carried and live-out values so the result is behaviour-preserving. Key properties: - Supports ascending (+1) and descending (-1) unit-step inductions, in both signed and unsigned iteration orderings, with direction-aware guard/latch predicates and end clamps. - Reuses the original loop for partition 0 and clones the rest, exposing per-partition value maps via getPartitionValue()/getPartitionValueMap(). - Lets callers drop the entry guard for a partition proven non-empty via avoidPartitionGuard(); provably-empty partitions are always skipped. - Patches the dominator tree and LoopInfo incrementally rather than rebuilding them. How to use: LoopSplitUtils LSU(L, LI, SE, DT); if (!LSU.isLegal()) // counted, bottom-tested, LCSSA, unit step return false; // Tile the iteration space in order; e.g. split [Start, End] at K: LSU.addPartition(Start, K - 1); // partition 0: [Start, K-1] LSU.addPartition(K, End); // partition 1: [K, End] LSU.split(); // After split(), query a cloned value in a given partition: Value *V1 = LSU.getPartitionValue(Orig, /*PartitionIndex=*/1); A new test pass, loop-split-test (opt -passes=loop-split-test with -loop-split-points=...), drives the utility for testing. Adds lit tests covering basic/multiple/four-partition splits, descending loops, reductions, empty-leading partitions, optional guards, and the per-partition value map.
fhahn
left a comment
There was a problem hiding this comment.
Please see inline for some code that looks like it might miscompile, crash or is not covered by existing tests.
It would have been good to try to break down the changes into smaller chunks, as the current size makes reviewing quite challenging.
It looks like there are a few pieces that could be added incrementally, like exit value support etc.
| // make_early_inc_range advances past each use before RewriteUse() unlinks | ||
| // it from Def's use-list, so the rewrite cannot invalidate the iteration. | ||
| if (EV.EscapesOutside) | ||
| for (Use &U : make_early_inc_range(EV.Def->uses())) |
There was a problem hiding this comment.
Constants no longer have use lists, so I'd expect this to crash when EV.Def is a constant.
| // it from Def's use-list, so the rewrite cannot invalidate the iteration. | ||
| if (EV.EscapesOutside) | ||
| for (Use &U : make_early_inc_range(EV.Def->uses())) | ||
| if (auto *User = dyn_cast<Instruction>(U.getUser())) |
There was a problem hiding this comment.
Do we have a test where the user is not an instruciton?
| // Repair outside uses before the carried-PHI seeds add new in-clone uses. | ||
| // make_early_inc_range advances past each use before RewriteUse() unlinks | ||
| // it from Def's use-list, so the rewrite cannot invalidate the iteration. | ||
| if (EV.EscapesOutside) |
There was a problem hiding this comment.
How does this handle cases where the live-out value is defined outside loop, and has uses before the loop? In those case they will get incorrectly rewritten to poison?
| ICmpInst::Predicate Pred = continuePredicate(Signed, Descending, | ||
| /*Inclusive=*/!LatchComparesPHI); |
There was a problem hiding this comment.
What about loops where the IV takes every value from start to the maxium value for the type? If LatchComparesPHI is true, we now created an infinite loop?
| if (S.OuterLoop) | ||
| S.OuterLoop->addBasicBlockToLoop(S.FinalExit, *LI); |
| if (!L->hasDedicatedExits() && | ||
| !formDedicatedExitBlocks(L, DT, LI, /*MSSAU=*/nullptr, | ||
| /*PreserveLCSSA=*/true)) | ||
| return false; |
|
Thanks for the review comments. I'll address the inline comments and add/update tests where needed. I agree the PR grew larger than expected, and I'll try to keep future changes more incremental and easier to review. |
…ting (llvm#205995) Introduce LoopSplitUtils, a utility that splits a counted loop into a chain of per-partition sub-loops covering contiguous slices of the original iteration space. Given a loop and a list of partition ranges, it clones the body per partition, guards each with an entry check that skips empty partitions, clamps each latch to its slice, and rebuilds SSA for loop-carried and live-out values so the result is behaviour-preserving. Key properties: - Supports ascending (+1) and descending (-1) unit-step inductions, in both signed and unsigned iteration orderings, with direction-aware guard/latch predicates and end clamps. - Reuses the original loop for partition 0 and clones the rest, exposing per-partition value maps via getPartitionValue()/getPartitionValueMap(). - Lets callers drop the entry guard for a partition proven non-empty via avoidPartitionGuard(); provably-empty partitions are always skipped. - Patches the dominator tree and LoopInfo incrementally rather than rebuilding them. How to use: LoopSplitUtils LSU(L, LI, SE, DT); if (!LSU.isLegal()) // counted, bottom-tested, LCSSA, unit step return false; // Tile the iteration space in order; e.g. split [Start, End] at K: LSU.addPartition(Start, K - 1); // partition 0: [Start, K-1] LSU.addPartition(K, End); // partition 1: [K, End] LSU.split(); // After split(), query a cloned value in a given partition: Value *V1 = LSU.getPartitionValue(Orig, /*PartitionIndex=*/1); A new test pass, loop-split-test (opt -passes=loop-split-test with -loop-split-points=...), drives the utility for testing. Adds lit tests covering basic/multiple/four-partition splits, descending loops, reductions, empty-leading partitions, optional guards, and the per-partition value map.
fhahn
left a comment
There was a problem hiding this comment.
Thanks for the review comments. I'll address the inline comments and add/update tests where needed. I agree the PR grew larger than expected, and I'll try to keep future changes more incremental and easier to review.
It might would have been worth considering reverting so there’s less time pressure to and the fixes.
Introduce LoopSplitUtils, a utility that splits a counted loop into a chain of per-partition sub-loops covering contiguous slices of the original iteration space. Given a loop and a list of partition ranges, it clones the body per partition, guards each with an entry check that skips empty partitions, clamps each latch to its slice, and rebuilds SSA for loop-carried and live-out values so the result is behaviour-preserving.
Key properties:
How to use:
LoopSplitUtils LSU(L, LI, SE, DT);
if (!LSU.isLegal()) // counted, bottom-tested, LCSSA, unit step
return false;
// Tile the iteration space in order; e.g. split [Start, End] at K:
LSU.addPartition(Start, K - 1); // partition 0: [Start, K-1]
LSU.addPartition(K, End); // partition 1: [K, End]
LSU.split();
// After split(), query a cloned value in a given partition:
Value *V1 = LSU.getPartitionValue(Orig, /PartitionIndex=/1);
A new test pass, loop-split-test (opt -passes=loop-split-test with -loop-split-points=...), drives the utility for testing. Adds lit tests covering basic/multiple/four-partition splits, descending loops, reductions, empty-leading partitions, optional guards, and the per-partition value map.