[clang] [llvm] [mlir] [openmp] [LoopTiling][Clang][OpenMP] Canonical Intra-tile Loops (PR #191114)
Meinersbur wrote:
> In the min-bound default, it is obvious to see predicate not getting emitted
> resulting in incorrect semantics. So, either way, we rely on the marker to be
> handled properly.
So we have two versions of the same loop:
```cpp
for (int i_intratile = i_floor; i_intratile < min(n, i_floor + i_tilesize);
++i_intratile)
Body(i_intratile);
```
and
```cpp
for (int i_intratile = i_floor; i_intratile < i_floor + i_tilesize;
++i_intratile)
if (i_intratile < n)
Body(i_intratile);
```
I think we agree that either version is correct. The attribute is supposed to
convey that one version can be transformed into the other, NOT that something
has to be added in order to make it semantically correct. Most
`__attribute__`/`[[attribute]]` are not used for correctness.
For reasons stated before, I would take the first form as starting point, then
attribute it:
```cpp
AttributedStmt: __attribute__(("The following loop can be reinterpreted as a
canonical loop lb=i_floor, ub=i_floor+i_tilesize with predicate if (i_intratile
< n) inserted before the body"))
ForStmt: for (int i_intratile = i_floor; i_intratile < min(n, i_floor +
i_tilesize); ++i_intratile)
CompoundStmt: Body(i_intratile);
```
If encountering in CGStmt: Ignore the AttributedStmt, generate code from ForStmt
If encountering in checkOpenMPLoop: Use the stated lb/ub[^2] instead of trying
to extract it from ForStmt[^1]. Remember the we will have to insert a predicate
in the innermost loop's body (like PreInit is what we have to remember to
insert outside the outermost loop).
[^1]: OpenMP will use the result from `LoopHelper`/`checkOpenMPLoop` anyway and
not even look at the ForStmt after `checkOpenMPLoop`.
[^2]: I am not sure whether it makes sense to store lb/ub/predicate directly in
the AttributedStmt. It could also be a hint "I guarantee that the Stmt I am
attributing as the form `ForStmt(i = lb; i < min(global_max, ub); ++i)`. Use
lb/ub/i < global_max for building LoopHelper"
https://github.com/llvm/llvm-project/pull/191114
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [openmp] [LoopTiling][Clang][OpenMP] Canonical Intra-tile Loops (PR #191114)
loopacino wrote: > That is, without additional code in CGStmt that catches the AttributedStmt, a > predicate is emitted. I would have done it the other way around: Make the > default AST emit the vectorizable code (so no additional handling in CGStmt > is needed; AttributedStmt can just be ignored, no change in output), and make > `checkOpenMPLoop` recognize the AttributedStmt: If present, it knows it can > convert the non-canonical ForStmt condition into a canonical one by > remembering it still has to emit a predicate around the body in > TransformedStmt. The main benefit is that this is more localized to the > handling of OpenMP loops than dragging its complexity into the > general-purpose CodeGen. Downside might be that the predicate needs to be > remembered through the composition of multiple loop transformations (altough > I think this approach might have the same problem; tests for it are missing). > This is not how it needs to be done, but I would like to know why not. After spending some time comparing the two, few points came up. Initially, it seemed that the current (canonical by-default) is safer "if any future directive misses out the marker". It turns out that in both the approaches, there exist a failing case if the marker is missed/unrecognized. In the current approach, the inner-loop is re-emitted and the loop runs too many times (was also a bug handled before posting the recent changes). In the min-bound default, it is obvious to see predicate not getting emitted resulting in incorrect semantics. So, either way, we rely on the marker to be handled properly. In the min-bound by-default, if we consider the case where the tiled loop isn't consumed–results in not touching the CodeGen. However, min-bound by-default wouldn't allow us to remove CodeGen impact altogether, especially in case where we would have to emit the predicate later; also unfolding min-bound later is less convenient than pushing predicate to min-bound (in canonical by-default, not a deal-breaker I agree). The point is that both the designs need that emit-side handling. So the "no CodeGen change" benefit is real, but it only covers the standalone case, not the consuming cases. Keeping the canonicalized by-default seems to be a valid-rectangular, self-consistent (transformed AST) form in every context–standalone, 2-level tiling, -nesting tiling, etc. I would rather keep the always-valid form. If keeping OpenMP checks into `EmitForStmt` is a main concern, we may reconsider the emitting order from canonical by-default to min-bound by-default. Let me know your opinions. https://github.com/llvm/llvm-project/pull/191114 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [openmp] [LoopTiling][Clang][OpenMP] Canonical Intra-tile Loops (PR #191114)
https://github.com/Meinersbur edited https://github.com/llvm/llvm-project/pull/191114 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [openmp] [LoopTiling][Clang][OpenMP] Canonical Intra-tile Loops (PR #191114)
@@ -215,6 +215,9 @@ int main() {
// CHECK-NEXT: i=2 j=1
// CHECK-NEXT: [B] iterator dtor
// CHECK-NEXT: [A] iterator dtor
+// CHECK-NEXT: [A] iterator advance: 0 += 3
+// CHECK-NEXT: [A] iterator move assign
+// CHECK-NEXT: [A] iterator dtor
Meinersbur wrote:
Nothing wrong to change side-effects if necessary, but why does this change?
https://github.com/llvm/llvm-project/pull/191114
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [openmp] [LoopTiling][Clang][OpenMP] Canonical Intra-tile Loops (PR #191114)
@@ -14983,35 +14985,118 @@ StmtResult
SemaOpenMP::ActOnOpenMPTileDirective(ArrayRef Clauses,
FloorIndVars[I] = FloorCntDecl;
}
-// Iteration variable for the tile (i.e. inner) loop.
+// Logical iteration variable for the tile loop. Retains the meaning of
+// the original logical iteration number (floor_iv + tile_cnt) so that
+// LoopHelper.Updates can derive the original loop variable unchanged.
{
- std::string TileCntName =
+ std::string TileIVName =
(Twine(".tile_") + llvm::utostr(I) + ".iv." + OrigVarName).str();
- // Reuse the iteration variable created by checkOpenMPLoop. It is also
- // used by the expressions to derive the original iteration variable's
- // value from the logical iteration number.
- auto *TileCntDecl = cast(IterVarRef->getDecl());
- TileCntDecl->setDeclName(
- &SemaRef.PP.getIdentifierTable().get(TileCntName));
- TileIndVars[I] = TileCntDecl;
+ auto *TileIVDecl = cast(IterVarRef->getDecl());
+
TileIVDecl->setDeclName(&SemaRef.PP.getIdentifierTable().get(TileIVName));
+ TileIndVars[I] = TileIVDecl;
+}
+
+// Loop counter for the rectangular tile loop [0, TileSize).
+{
+ std::string TileCntName =
+ (Twine(".tile.cnt.") + llvm::utostr(I) + ".iv." + OrigVarName).str();
+ VarDecl *TileCntDecl =
+ buildVarDecl(SemaRef, {}, CntTy, TileCntName, nullptr, OrigCntVar);
+ TileCntVars[I] = TileCntDecl;
}
addLoopPreInits(Context, LoopHelper, LoopStmts[I], OriginalInits[I],
PreInits);
+
+// Declare the logical tile IV in PreInits so it is in scope for the
+// entire loop nest (it will be assigned in each tile loop body).
+{
+ Decl *TileIVDeclPtr = TileIndVars[I];
+ PreInits.push_back(new (Context) DeclStmt(
+ DeclGroupRef::Create(Context, &TileIVDeclPtr, 1), {}, {}));
+}
}
// Once the original iteration values are set, append the innermost body.
Stmt *Inner = Body;
+ // Build a combined validity predicate that guards the innermost body.
+ // For each tiled dimension, check that the logical iteration number
+ // (.tile.iv) is within the original trip count. This is required because the
+ // tile loop now has rectangular (constant) bounds and may overshoot on the
+ // remainder tile. The predicate is: .tile.iv.0 < N0 && .tile.iv.1 < N1 ...
+ //
+ // Optimization: if every dimension's trip count is a compile-time constant
+ // that is evenly divisible by the corresponding tile size (also a constant),
+ // then the remainder tile is empty and the predicate is trivially true.
+ //
+ bool PredicateNeeded = false;
+ for (unsigned I = 0; I < NumLoops; ++I) {
+Expr *TSExpr = SizesClause->getSizesRefs()[I];
+Expr *NExpr = LoopHelpers[I].NumIterations;
+Expr::EvalResult TSResult;
+Expr::EvalResult NResult;
+if (TSExpr->EvaluateAsInt(TSResult, Context) &&
+NExpr->EvaluateAsInt(NResult, Context)) {
+ llvm::APSInt TileVal = TSResult.Val.getInt();
+ llvm::APSInt TripVal = NResult.Val.getInt();
+ if (TileVal.isStrictlyPositive() && TripVal.srem(TileVal).isZero())
+continue;
+}
+PredicateNeeded = true;
+break;
+ }
+
+ if (PredicateNeeded) {
+Expr *CombinedPred = nullptr;
+for (unsigned I = 0; I < NumLoops; ++I) {
+ auto *OrigCntVar = cast(LoopHelpers[I].Counters[0]);
+ QualType IVTy = LoopHelpers[I].NumIterations->getType();
+ Expr *FloorRef = buildDeclRefExpr(SemaRef, FloorIndVars[I], IVTy,
+OrigCntVar->getExprLoc());
+ Expr *TileCntRef = buildDeclRefExpr(SemaRef, TileCntVars[I], IVTy,
+ OrigCntVar->getExprLoc());
+ ExprResult Sum = SemaRef.BuildBinOp(CurScope, OrigCntVar->getExprLoc(),
+ BO_Add, FloorRef, TileCntRef);
+ if (!Sum.isUsable())
+return StmtError();
+ ExprResult DimPred =
+ SemaRef.BuildBinOp(CurScope, OrigCntVar->getExprLoc(), BO_LT,
+ Sum.get(), LoopHelpers[I].NumIterations);
+ if (!DimPred.isUsable())
+return StmtError();
+ if (CombinedPred) {
+ExprResult Combined =
+SemaRef.BuildBinOp(CurScope, OrigCntVar->getExprLoc(), BO_LAnd,
+ CombinedPred, DimPred.get());
+if (!Combined.isUsable())
+ return StmtError();
+CombinedPred = Combined.get();
+ } else {
+CombinedPred = DimPred.get();
+ }
+}
+// Use the source body's location for the synthetic if-statement and
+// AttributedStmt so any later diagnostic points at the original body
+// location.
+SourceLocation SyntheticLoc =
+Body ? Body->getBeginLoc() : Inner->getBeginLoc();
+Stmt *PredIf = IfStmt::Create(
+Context, SyntheticLoc, IfStatementKind::Ordinary, nullptr, nul
[clang] [llvm] [mlir] [openmp] [LoopTiling][Clang][OpenMP] Canonical Intra-tile Loops (PR #191114)
https://github.com/Meinersbur edited https://github.com/llvm/llvm-project/pull/191114 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [openmp] [LoopTiling][Clang][OpenMP] Canonical Intra-tile Loops (PR #191114)
@@ -10,12 +10,48 @@
//
//===--===//
-#include "clang/AST/ASTContext.h"
#include "clang/AST/StmtOpenMP.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Attr.h"
+#include "clang/AST/Stmt.h"
using namespace clang;
using namespace llvm::omp;
+// True if the attributed statement carries the internal `omp tile` intra-tile
+// body-guard marker.
+static bool hasOMPInvariantPredicateBound(const AttributedStmt *AS) {
+ for (const Attr *A : AS->getAttrs())
+if (isa(A))
+ return true;
+ return false;
+}
+
+// Like `Stmt::IgnoreContainers`, but also treats the `omp tile` body-guard
+static Stmt *IgnoreContainersAndOMPTileBodyGuard(Stmt *S) {
Meinersbur wrote:
```suggestion
static Stmt *ignoreContainersAndOMPTileBodyGuard(Stmt *S) {
```
Start functions with lower case letters:
https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
Not all functions have been renamed to follow the updated coding style, but for
new functions use the current coding standard.
https://github.com/llvm/llvm-project/pull/191114
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [openmp] [LoopTiling][Clang][OpenMP] Canonical Intra-tile Loops (PR #191114)
https://github.com/Meinersbur commented: Please remove "[LoopTiling]" tag from the title, it is not a component of Clang. I would expect more testing in transform/tile, for instance: 1. Multi-dimensional tiling 2. collapse that only affects some, but not all of the intratile-loops of a multi-dimensional tiling 3. loops on user-defined iterators (like iterfor.cpp, but also collapsing some of the intratile-loops) 4. A check whether vectorization takes place on a non-affected innermost intra-tile loop is taking place 5. Composition of multiple loop transformation (tile and others) 6. ... There is no explanation of the overall design of this change which makes it very hard to review. The PR summary as well as a big comments at an central location need to explain it, where other comments that mention "omp-tile body-guard marker" can point to. Someone who encounters it while reading non-OpenMP-related code will be puzzled by what it is about. For instance, it should sketch how the AST structure looks like and what the semantics of OMPInvariantPredicateBoundAttr are. I tried to understand it anyway and here is what I got: 1. If all loops are known-multiples of the tile size, no predicate is emitted (Review comment: could we do it per-loop?) 2. The intratile ForStmt condition is emitted without partial tile check. That is, without additional code, it will overshoot the iteration space 3. The body is surrounded with an IfStmt with an AttributedStmt of an OMPInvariantPredicateBoundAttr checking for the iteration space. That is, without additional code in CGStmt that catches the AttributedStmt, a predicate is emitted. I would have done it the other way around: Make the default AST emit the vectorizable code (so no additional handling in CGStmt is needed; AttributedStmt can just be ignored, no change in output), and make `checkOpenMPLoop` recognize the AttributedStmt: If present, it knows it can convert the non-canonical ForStmt condition into a canonical one by remembering it still has to emit a predicate around the body in TransformedStmt. The main benefit is that this is more localized to the handling of OpenMP loops than dragging its complexity into the general-purpose CodeGen. Downside might be that the predicate needs to be remembered through the composition of multiple loop transformations (altough I think this approach might have the same problem; tests for it are missing). https://github.com/llvm/llvm-project/pull/191114 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [openmp] [LoopTiling][Clang][OpenMP] Canonical Intra-tile Loops (PR #191114)
@@ -10,12 +10,48 @@ // //===--===// -#include "clang/AST/ASTContext.h" #include "clang/AST/StmtOpenMP.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Attr.h" +#include "clang/AST/Stmt.h" using namespace clang; using namespace llvm::omp; +// True if the attributed statement carries the internal `omp tile` intra-tile +// body-guard marker. Meinersbur wrote: Use `///`-comments for function documentaton https://github.com/llvm/llvm-project/pull/191114 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [openmp] [LoopTiling][Clang][OpenMP] Canonical Intra-tile Loops (PR #191114)
loopacino wrote: > Please create separate PRs for Clang and MLIR. These are separate > implementations don't need to be reviewed together. I am focusing on Clang in > this PR. > > None of the added test are actually requiring the intratile loops to be > canonical. That is, something where an surrounding loop-associated directives > affects a grid/floor loop as well as a intratile loop at the same time. For > instance: > > ```c++ > #pragma omp for collapse(2) > #pragma omp tile size(2) > for (int i = 0; i < n; ++i) >... > ``` > > or two-level tiling: > > ```c++ > #pragma omp tile sizes(3, 5) > #pragma omp tile size(2) > for (int i = 0; i < n; ++i) >... > ``` > > `IntraTileMustBeCanonical` is an ok-ish first heuristic, a refinement would > be to which of the tile loops are actually affected and thus would need to be > canonical. For instance: > > ```c++ > #pragma omp for collapse(3) > #pragma omp tile size(2, 3) > for (int i = 0; i < n; ++i) > for (int j = 0; j < n; ++j) >... > ``` > > here, the intratile-loop for `j` does not need to be canonical, but `i` does. > Because as you found out emitting the canonical version of the loop inhibits > vectorization, it would be quite important to eventually have this. > > Because Clang implements loop-transformations using AST transforms, I think > it would be easier and more accurate if we always emit the canonical form, > but emit a marker that allows to optimize the canonical form to the > non-canonical one later, as suggested > [here](https://github.com/llvm/llvm-project/pull/191114#issuecomment-4313291974). > For instance, it could also emit an alternatve AST depending on the > situation we are in, like > `OMPCanonicalLoopNestTransformationDirective::getTransformed()` does. Imagine > such an AST: > > ``` > OMPLoopAlternative > `- getCanonical(): ForStmt // Predicated version when canonical version is > needed >`- for (int i = 0; i < n; ++) > `- if (i < .floor.iv+tilesize) > `- getOptimized(): ForStmt // Vectorizable version when not used as an > affected loop >`- for (int i = 0; i < min(n, .floor.iv+tilesize; ++) > ``` > > Storing both versions at the same time might not be the best solution due to > the overhead of storing both. Instance, it could just add a marker (such as > `AttributedStmt`) to the loop which says that the ForStmt can be emitted as > the optimized one at CodeGen, i.e.: > > ``` > for (int i = 0; i < n; ++) > `- AttributedStmt __attribute__((__invariant_pred_bound)) // States that the > RHS is invariant to the loop using the LHS as IV > `- if (i < .floor.iv+tilesize) > `- ... > ``` > > is emitted as > > ``` > for (int i = 0; i < min(n, .floor.iv+tilesize); ++) > `- ... > ``` > > I think both approaches are viable, but we don't need to start with a > `IntraTileMustBeCanonical` if eventually we are going to do the second. Which > approach would you prefer? > > When I try this out, I get 3 failing tests: > > ``` > Failed Tests (3): > libomp :: transform/tile/foreach.cpp > libomp :: transform/tile/intfor.c > libomp :: transform/tile/iterfor.cpp > ``` > > Test updates in this PR are adding more iterator evaluations, which assume is > because mostly because fewer invariant values are precomputed? Ideally, If > `IntraTileMustBeCanonical` is false it generates the same code. This should > actually be easier to do with an `IntraTileMustBeCanonical` heursitic than > with the marker approach. 1. Reverted MLIR changes 2. As suggested, went with the marker-based approach. 3. All tests pass now. https://github.com/llvm/llvm-project/pull/191114 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [openmp] [LoopTiling][Clang][OpenMP] Canonical Intra-tile Loops (PR #191114)
@@ -14983,35 +14994,113 @@ StmtResult
SemaOpenMP::ActOnOpenMPTileDirective(ArrayRef Clauses,
FloorIndVars[I] = FloorCntDecl;
}
-// Iteration variable for the tile (i.e. inner) loop.
+// Logical iteration variable for the tile loop. Retains the meaning of
+// the original logical iteration number (floor_iv + tile_cnt) so that
+// LoopHelper.Updates can derive the original loop variable unchanged.
{
- std::string TileCntName =
+ std::string TileIVName =
(Twine(".tile_") + llvm::utostr(I) + ".iv." + OrigVarName).str();
- // Reuse the iteration variable created by checkOpenMPLoop. It is also
- // used by the expressions to derive the original iteration variable's
- // value from the logical iteration number.
- auto *TileCntDecl = cast(IterVarRef->getDecl());
- TileCntDecl->setDeclName(
- &SemaRef.PP.getIdentifierTable().get(TileCntName));
- TileIndVars[I] = TileCntDecl;
+ auto *TileIVDecl = cast(IterVarRef->getDecl());
+
TileIVDecl->setDeclName(&SemaRef.PP.getIdentifierTable().get(TileIVName));
+ TileIndVars[I] = TileIVDecl;
+}
+
+// Loop counter for the rectangular tile loop [0, TileSize).
+if (IntraTileMustBeCanonical) {
+ std::string TileCntName =
+ (Twine(".tile.cnt.") + llvm::utostr(I) + ".iv." + OrigVarName).str();
+ VarDecl *TileCntDecl =
+ buildVarDecl(SemaRef, {}, CntTy, TileCntName, nullptr, OrigCntVar);
+ TileCntVars[I] = TileCntDecl;
}
addLoopPreInits(Context, LoopHelper, LoopStmts[I], OriginalInits[I],
PreInits);
+
+// Declare the logical tile IV in PreInits so it is in scope for the
+// entire loop nest (it will be assigned in each tile loop body).
+if (IntraTileMustBeCanonical) {
+ Decl *TileIVDeclPtr = TileIndVars[I];
+ PreInits.push_back(new (Context) DeclStmt(
+ DeclGroupRef::Create(Context, &TileIVDeclPtr, 1), {}, {}));
+}
}
// Once the original iteration values are set, append the innermost body.
Stmt *Inner = Body;
+ // Build a combined validity predicate that guards the innermost body.
+ // For each tiled dimension, check that the logical iteration number
+ // (.tile.iv) is within the original trip count. This is required because the
+ // tile loop now has rectangular (constant) bounds and may overshoot on the
+ // remainder tile. The predicate is: .tile.iv.0 < N0 && .tile.iv.1 < N1 ...
+ //
+ // Optimization: if every dimension's trip count is a compile-time constant
+ // that is evenly divisible by the corresponding tile size (also a constant),
+ // then the remainder tile is empty and the predicate is trivially true.
+ //
+ // The min-bounded path clamps the tile loop's upper bound directly so no
+ // predicate is needed.
+ if (IntraTileMustBeCanonical) {
+bool PredicateNeeded = false;
+for (unsigned I = 0; I < NumLoops; ++I) {
+ Expr *TSExpr = SizesClause->getSizesRefs()[I];
+ Expr *NExpr = LoopHelpers[I].NumIterations;
+ bool TSConst =
+ !TSExpr->containsErrors() && TSExpr->isIntegerConstantExpr(Context);
+ bool NConst = NExpr->isIntegerConstantExpr(Context);
loopacino wrote:
Addressed, thanks.
https://github.com/llvm/llvm-project/pull/191114
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [openmp] [LoopTiling][Clang][OpenMP] Canonical Intra-tile Loops (PR #191114)
@@ -56,7 +56,7 @@ extern "C" void foo4() {
extern "C" void foo5() {
-#pragma omp for collapse(3)
+#pragma omp for collapse(2)
loopacino wrote:
Addressed. Also a added a new test to trigger such cases
clang/test/OpenMP/tile_canonical_required_codegen.cpp
https://github.com/llvm/llvm-project/pull/191114
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [openmp] [LoopTiling][Clang][OpenMP] Canonical Intra-tile Loops (PR #191114)
@@ -,18 +6651,47 @@ OpenMPIRBuilder::tileLoops(DebugLoc DL,
ArrayRef Loops,
Value *Shift =
Builder.CreateAdd(Scale, TileLoop->getIndVar(), {}, /*HasNUW=*/true);
OrigIndVar->replaceAllUsesWith(Shift);
+ShiftValues.push_back(Shift);
+ }
+
+ // Build a validity predicate: for each tiled dimension, check that the
+ // computed original index is within the original trip count. This guards
+ // against executing out-of-bounds iterations in the remainder tile.
+ Value *Pred = nullptr;
+ for (int i = 0; i < NumLoops; ++i) {
+Value *DimPred = Builder.CreateICmpULT(ShiftValues[i], OrigTripCounts[i],
+ "omp_tile" + Twine(i) +
".inbounds");
+Pred = Pred ? Builder.CreateAnd(Pred, DimPred) : DimPred;
}
+ // Insert the conditional guard: split the body flow so that out-of-bounds
+ // iterations skip directly to a merge block before the tile latch.
+ BasicBlock *TileBodyBB = Builder.GetInsertBlock();
+ Instruction *BodyTerm = TileBodyBB->getTerminator();
+ BasicBlock *BodySucc = cast(BodyTerm)->getSuccessor(0);
+ BasicBlock *TileLatch = Result.back()->getLatch();
+
+ BasicBlock *MergeBB =
+ BasicBlock::Create(F->getContext(), "omp_tile.body.merge", F, TileLatch);
+ Builder.SetInsertPoint(MergeBB);
+ Builder.CreateBr(TileLatch);
+
+ // Redirect the body chain's exit from the tile latch to the merge block.
+ for (BasicBlock *PredBB :
llvm::make_early_inc_range(predecessors(TileLatch)))
+if (PredBB != MergeBB)
+ PredBB->getTerminator()->replaceUsesOfWith(TileLatch, MergeBB);
+
+ // Replace the tile body's unconditional branch with a conditional one.
+ BodyTerm->eraseFromParent();
+ Builder.SetInsertPoint(TileBodyBB);
+ Builder.CreateCondBr(Pred, BodySucc, MergeBB);
+
// Remove unused parts of the original loops.
removeUnusedBlocksFromParent(OldControlBBs);
for (CanonicalLoopInfo *L : Loops)
L->invalidate();
-#ifndef NDEBUG
- for (CanonicalLoopInfo *GenL : Result)
-GenL->assertOK();
-#endif
loopacino wrote:
Reverted the MLIR-changes to be raised in a separate PR.
https://github.com/llvm/llvm-project/pull/191114
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [mlir] [openmp] [LoopTiling][Clang][OpenMP] Canonical Intra-tile Loops (PR #191114)
https://github.com/loopacino edited https://github.com/llvm/llvm-project/pull/191114 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
