Skip to content

[CIR] Skip generation of a continue block when flattening TernaryOp #142165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mmha
Copy link
Contributor

@mmha mmha commented May 30, 2025

We used to insert a continue Block at the end of a flattened ternary op that only contained a branch to the remaing operation of the remaining Block. This patch removes that continue block and changes the true/false blocks to directly jump to the remaining ops.

With this patch the CIR now generates exactly the same LLVM IR as the original codegen.

This upstreams llvm/clangir#1651.

We used to insert a continue Block at the end of a flattened ternary op that only contained a branch to the remaing operation of the remaining Block. This patch removes that continue block and changes the true/false blocks to directly jump to the remaining ops.

With this patch the CIR now generates exactly the same LLVM IR as the original codegen.

This upstreams llvm/clangir#1651.
@mmha mmha requested review from erichkeane and andykaylor May 30, 2025 14:57
@mmha mmha requested review from lanza and bcardosolopes as code owners May 30, 2025 14:57
@llvmbot llvmbot added clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project labels May 30, 2025
@llvmbot
Copy link
Member

llvmbot commented May 30, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clangir

Author: Morris Hafner (mmha)

Changes

We used to insert a continue Block at the end of a flattened ternary op that only contained a branch to the remaing operation of the remaining Block. This patch removes that continue block and changes the true/false blocks to directly jump to the remaining ops.

With this patch the CIR now generates exactly the same LLVM IR as the original codegen.

This upstreams llvm/clangir#1651.


Full diff: https://github.com/llvm/llvm-project/pull/142165.diff

3 Files Affected:

  • (modified) clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp (+15-16)
  • (modified) clang/test/CIR/Lowering/ternary.cir (-2)
  • (modified) clang/test/CIR/Transforms/ternary.cir (-4)
diff --git a/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp b/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp
index 26e5c0572f12e..6081a436f5c29 100644
--- a/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp
+++ b/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp
@@ -16,9 +16,11 @@
 #include "mlir/IR/Block.h"
 #include "mlir/IR/Builders.h"
 #include "mlir/IR/PatternMatch.h"
+#include "mlir/IR/ValueRange.h"
 #include "mlir/Support/LogicalResult.h"
 #include "mlir/Transforms/DialectConversion.h"
 #include "mlir/Transforms/GreedyPatternRewriteDriver.h"
+#include "clang/AST/DeclBase.h"
 #include "clang/CIR/Dialect/IR/CIRDialect.h"
 #include "clang/CIR/Dialect/Passes.h"
 #include "clang/CIR/MissingFeatures.h"
@@ -491,15 +493,7 @@ class CIRTernaryOpFlattening : public mlir::OpRewritePattern<cir::TernaryOp> {
     Location loc = op->getLoc();
     Block *condBlock = rewriter.getInsertionBlock();
     Block::iterator opPosition = rewriter.getInsertionPoint();
-    Block *remainingOpsBlock = rewriter.splitBlock(condBlock, opPosition);
-    llvm::SmallVector<mlir::Location, 2> locs;
-    // Ternary result is optional, make sure to populate the location only
-    // when relevant.
-    if (op->getResultTypes().size())
-      locs.push_back(loc);
-    Block *continueBlock =
-        rewriter.createBlock(remainingOpsBlock, op->getResultTypes(), locs);
-    rewriter.create<cir::BrOp>(loc, remainingOpsBlock);
+    auto *remainingOpsBlock = rewriter.splitBlock(condBlock, opPosition);
 
     Region &trueRegion = op.getTrueRegion();
     Block *trueBlock = &trueRegion.front();
@@ -508,24 +502,29 @@ class CIRTernaryOpFlattening : public mlir::OpRewritePattern<cir::TernaryOp> {
     auto trueYieldOp = dyn_cast<cir::YieldOp>(trueTerminator);
 
     rewriter.replaceOpWithNewOp<cir::BrOp>(trueYieldOp, trueYieldOp.getArgs(),
-                                           continueBlock);
-    rewriter.inlineRegionBefore(trueRegion, continueBlock);
+                                           remainingOpsBlock);
+    rewriter.inlineRegionBefore(trueRegion, remainingOpsBlock);
 
-    Block *falseBlock = continueBlock;
     Region &falseRegion = op.getFalseRegion();
+    Block *falseBlock = &falseRegion.front();
 
-    falseBlock = &falseRegion.front();
     mlir::Operation *falseTerminator = falseRegion.back().getTerminator();
     rewriter.setInsertionPointToEnd(&falseRegion.back());
     auto falseYieldOp = dyn_cast<cir::YieldOp>(falseTerminator);
     rewriter.replaceOpWithNewOp<cir::BrOp>(falseYieldOp, falseYieldOp.getArgs(),
-                                           continueBlock);
-    rewriter.inlineRegionBefore(falseRegion, continueBlock);
+                                           remainingOpsBlock);
+    rewriter.inlineRegionBefore(falseRegion, remainingOpsBlock);
 
     rewriter.setInsertionPointToEnd(condBlock);
     rewriter.create<cir::BrCondOp>(loc, op.getCond(), trueBlock, falseBlock);
 
-    rewriter.replaceOp(op, continueBlock->getArguments());
+    if (auto rt = op.getResultTypes(); rt.size()) {
+      iterator_range args = remainingOpsBlock->addArguments(rt, op.getLoc());
+      SmallVector<mlir::Value, 2> values;
+      llvm::copy(args, std::back_inserter(values));
+      rewriter.replaceOpUsesWithinBlock(op, values, remainingOpsBlock);
+    }
+    rewriter.eraseOp(op);
 
     // Ok, we're done!
     return mlir::success();
diff --git a/clang/test/CIR/Lowering/ternary.cir b/clang/test/CIR/Lowering/ternary.cir
index 247c6ae3a1e17..c2226cd92ece7 100644
--- a/clang/test/CIR/Lowering/ternary.cir
+++ b/clang/test/CIR/Lowering/ternary.cir
@@ -25,6 +25,4 @@ module  {
 // LLVM:   br label %[[M]]
 // LLVM: [[M]]:
 // LLVM:   [[R:%[[:alnum:]]+]] = phi i32 [ 1, %[[B2]] ], [ 0, %[[B1]] ]
-// LLVM:   br label %[[B3:[[:alnum:]]+]]
-// LLVM: [[B3]]:
 // LLVM:   ret i32 [[R]]
diff --git a/clang/test/CIR/Transforms/ternary.cir b/clang/test/CIR/Transforms/ternary.cir
index 67ef7f95a6b52..0c22268495697 100644
--- a/clang/test/CIR/Transforms/ternary.cir
+++ b/clang/test/CIR/Transforms/ternary.cir
@@ -37,8 +37,6 @@ module {
 // CHECK:    %6 = cir.const #cir.int<5> : !s32i
 // CHECK:    cir.br ^bb3(%6 : !s32i)
 // CHECK:  ^bb3(%7: !s32i):  // 2 preds: ^bb1, ^bb2
-// CHECK:    cir.br ^bb4
-// CHECK:  ^bb4:  // pred: ^bb3
 // CHECK:    cir.store %7, %1 : !s32i, !cir.ptr<!s32i>
 // CHECK:    %8 = cir.load %1 : !cir.ptr<!s32i>, !s32i
 // CHECK:    cir.return %8 : !s32i
@@ -60,8 +58,6 @@ module {
 // CHECK: ^bb2:  // pred: ^bb0
 // CHECK:   cir.br ^bb3
 // CHECK: ^bb3:  // 2 preds: ^bb1, ^bb2
-// CHECK:   cir.br ^bb4
-// CHECK: ^bb4:  // pred: ^bb3
 // CHECK:   cir.return
 // CHECK: }
 

Copy link
Contributor

@andykaylor andykaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants