llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clangir
Author: David Rivera (RiverDave)
<details>
<summary>Changes</summary>
mem2reg gives up on a slot if any use lives in a nested region whose parent op
doesn't implement PromotableRegionOpInterface. No CIR op implements it. So this
doesn't promote:
```mlir
%a = cir.alloca "a" align(4) : !cir.ptr<!s32i>
%c = cir.const #cir.int<42> : !s32i
cir.store %c, %a : !s32i, !cir.ptr<!s32i>
cir.if %cond {
%v = cir.load %a : !cir.ptr<!s32i>, !s32i
cir.call @<!-- -->use(%v) : (!s32i) -> ()
}
```
You have to run -cir-flatten-cfg first, which is why mem2reg.cir does. That's a
lot of destruction for a value that never leaves the region.
This implements the interface for cir.if and cir.scope. Both regions are
entered straight from before the op, so they see the reaching definition
unchanged. Nothing to merge, nothing to yield.
Stores inside a region are refused. A new definition has to exit through a
result, cir.if has none, cir.scope has one it may already be using. Supporting
that means giving those ops results. Separate patch.
My use case specifically:
I'm propagating constants into CUDA kernel arguments.[ Every launch site sits
inside a cir.if, the __cudaPushCallConfiguration
guard](https://github.com/llvm/llvm-project/blob/5bf59e2c4b54a85e4e7f0e188b99061beb2708f6/clang/lib/CIR/CodeGen/CIRGenCUDARuntime.cpp#L33),
so the argument reaching the device stub call site is always a cir.load. In
particular this allows other MLIR passes like sccp, determine if such value is
in constant form through the call graph. So the argument flow would be like
this: mem2reg turns it into the block argument, sccp turns that into a
cir.const, and you get the constant at the call site.
Claude assisted with this patch.
---
Full diff: https://github.com/llvm/llvm-project/pull/215780.diff
3 Files Affected:
- (modified) clang/include/clang/CIR/Dialect/IR/CIROps.td (+2)
- (modified) clang/lib/CIR/Dialect/IR/CIRMemorySlot.cpp (+52)
- (added) clang/test/CIR/Transforms/mem2reg-regions.cir (+129)
``````````diff
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td
b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index 599ea50c85267..75aa59a80dddf 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -985,6 +985,7 @@ def CIR_ReturnOp : CIR_Op<"return", [
def CIR_IfOp : CIR_Op<"if", [
DeclareOpInterfaceMethods<RegionBranchOpInterface, ["getSuccessorInputs"]>,
+ DeclareOpInterfaceMethods<PromotableRegionOpInterface>,
RecursivelySpeculatable, AutomaticAllocationScope, NoRegionArguments
]> {
let summary = "the if-then-else operation";
@@ -1277,6 +1278,7 @@ def CIR_ResumeFlatOp : CIR_Op<"resume.flat", [
def CIR_ScopeOp : CIR_Op<"scope", [
DeclareOpInterfaceMethods<RegionBranchOpInterface, ["getSuccessorInputs"]>,
+ DeclareOpInterfaceMethods<PromotableRegionOpInterface>,
RecursivelySpeculatable, AutomaticAllocationScope, NoRegionArguments,
RecursiveMemoryEffects
]> {
diff --git a/clang/lib/CIR/Dialect/IR/CIRMemorySlot.cpp
b/clang/lib/CIR/Dialect/IR/CIRMemorySlot.cpp
index d6de6b6e80799..ca8260ee673f6 100644
--- a/clang/lib/CIR/Dialect/IR/CIRMemorySlot.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRMemorySlot.cpp
@@ -187,3 +187,55 @@ DeletionKind cir::CastOp::removeBlockingUses(
const SmallPtrSetImpl<OpOperand *> &blockingUses, OpBuilder &builder) {
return DeletionKind::Delete;
}
+
+//===----------------------------------------------------------------------===//
+// Interfaces for IfOp
+//===----------------------------------------------------------------------===//
+
+bool cir::IfOp::isRegionPromotable(const MemorySlot &slot, Region *region,
+ bool hasValueStores) {
+ // A definition produced inside a region has to leave the operation through
+ // one of its results, and cir.if has no result to receive it.
+ return !hasValueStores;
+}
+
+void cir::IfOp::setupPromotion(
+ const MemorySlot &slot, Value reachingDef, bool hasValueStores,
+ llvm::SmallMapVector<Region *, Value, 2> ®ionsToProcess) {
+ // Exactly one region executes, exactly once, entered from before the op, so
+ // both see the same reaching definition.
+ regionsToProcess.insert({&getThenRegion(), reachingDef});
+ regionsToProcess.insert({&getElseRegion(), reachingDef});
+}
+
+Value cir::IfOp::finalizePromotion(
+ const MemorySlot &slot, Value reachingDef, bool hasValueStores,
+ const llvm::DenseMap<Block *, Value> &reachingAtBlockEnd,
+ OpBuilder &builder) {
+ assert(!hasValueStores && "cir.if cannot yield a new definition");
+ return reachingDef;
+}
+
+//===----------------------------------------------------------------------===//
+// Interfaces for ScopeOp
+//===----------------------------------------------------------------------===//
+
+bool cir::ScopeOp::isRegionPromotable(const MemorySlot &slot, Region *region,
+ bool hasValueStores) {
+ // cir.scope yields at most one value, which it may already be using.
+ return !hasValueStores;
+}
+
+void cir::ScopeOp::setupPromotion(
+ const MemorySlot &slot, Value reachingDef, bool hasValueStores,
+ llvm::SmallMapVector<Region *, Value, 2> ®ionsToProcess) {
+ regionsToProcess.insert({&getScopeRegion(), reachingDef});
+}
+
+Value cir::ScopeOp::finalizePromotion(
+ const MemorySlot &slot, Value reachingDef, bool hasValueStores,
+ const llvm::DenseMap<Block *, Value> &reachingAtBlockEnd,
+ OpBuilder &builder) {
+ assert(!hasValueStores && "cir.scope cannot yield a new definition");
+ return reachingDef;
+}
diff --git a/clang/test/CIR/Transforms/mem2reg-regions.cir
b/clang/test/CIR/Transforms/mem2reg-regions.cir
new file mode 100644
index 0000000000000..8f5b5d91bbf9b
--- /dev/null
+++ b/clang/test/CIR/Transforms/mem2reg-regions.cir
@@ -0,0 +1,129 @@
+// RUN: cir-opt %s -mem2reg -o - | FileCheck %s
+
+!s32i = !cir.int<s, 32>
+
+module {
+ // A slot whose only nested use is a load can be promoted without flattening
+ // the CFG first.
+ // CHECK-LABEL: cir.func @load_in_if
+ cir.func @load_in_if(%cond: !cir.bool) {
+ // CHECK-NOT: cir.alloca
+ // CHECK: %[[CONST:.*]] = cir.const #cir.int<42>
+ // CHECK: cir.if
+ // CHECK-NOT: cir.load
+ // CHECK: cir.call @use(%[[CONST]])
+ %0 = cir.alloca "a" align(4) : !cir.ptr<!s32i>
+ %1 = cir.const #cir.int<42> : !s32i
+ cir.store %1, %0 : !s32i, !cir.ptr<!s32i>
+ cir.if %cond {
+ %2 = cir.load %0 : !cir.ptr<!s32i>, !s32i
+ cir.call @use(%2) : (!s32i) -> ()
+ cir.yield
+ }
+ cir.return
+ }
+
+ // Both regions are entered with the same reaching definition.
+ // CHECK-LABEL: cir.func @load_in_both_regions
+ cir.func @load_in_both_regions(%cond: !cir.bool) {
+ // CHECK-NOT: cir.alloca
+ // CHECK: %[[CONST:.*]] = cir.const #cir.int<7>
+ // CHECK: cir.if
+ // CHECK: cir.call @use(%[[CONST]])
+ // CHECK: cir.call @use(%[[CONST]])
+ // CHECK-NOT: cir.load
+ %0 = cir.alloca "a" align(4) : !cir.ptr<!s32i>
+ %1 = cir.const #cir.int<7> : !s32i
+ cir.store %1, %0 : !s32i, !cir.ptr<!s32i>
+ cir.if %cond {
+ %2 = cir.load %0 : !cir.ptr<!s32i>, !s32i
+ cir.call @use(%2) : (!s32i) -> ()
+ cir.yield
+ } else {
+ %3 = cir.load %0 : !cir.ptr<!s32i>, !s32i
+ cir.call @use(%3) : (!s32i) -> ()
+ cir.yield
+ }
+ cir.return
+ }
+
+ // A store inside a region would have to leave the operation through a
result,
+ // which cir.if does not have, so the slot is left alone.
+ // CHECK-LABEL: cir.func @store_in_if
+ cir.func @store_in_if(%cond: !cir.bool) {
+ // CHECK: cir.alloca
+ // CHECK: cir.if
+ // CHECK: cir.store
+ // CHECK: cir.load
+ %0 = cir.alloca "a" align(4) : !cir.ptr<!s32i>
+ %1 = cir.const #cir.int<1> : !s32i
+ cir.store %1, %0 : !s32i, !cir.ptr<!s32i>
+ cir.if %cond {
+ %2 = cir.const #cir.int<2> : !s32i
+ cir.store %2, %0 : !s32i, !cir.ptr<!s32i>
+ cir.yield
+ }
+ %3 = cir.load %0 : !cir.ptr<!s32i>, !s32i
+ cir.call @use(%3) : (!s32i) -> ()
+ cir.return
+ }
+
+ // CHECK-LABEL: cir.func @load_in_scope
+ cir.func @load_in_scope(%cond: !cir.bool) {
+ // CHECK-NOT: cir.alloca
+ // CHECK: %[[CONST:.*]] = cir.const #cir.int<3>
+ // CHECK: cir.scope
+ // CHECK-NOT: cir.load
+ // CHECK: cir.call @use(%[[CONST]])
+ %0 = cir.alloca "a" align(4) : !cir.ptr<!s32i>
+ %1 = cir.const #cir.int<3> : !s32i
+ cir.store %1, %0 : !s32i, !cir.ptr<!s32i>
+ cir.scope {
+ %2 = cir.load %0 : !cir.ptr<!s32i>, !s32i
+ cir.call @use(%2) : (!s32i) -> ()
+ }
+ cir.return
+ }
+
+ // A cir.scope between the slot and a nested cir.if is threaded through.
+ // CHECK-LABEL: cir.func @load_in_if_inside_scope
+ cir.func @load_in_if_inside_scope(%cond: !cir.bool) {
+ // CHECK-NOT: cir.alloca
+ // CHECK: %[[CONST:.*]] = cir.const #cir.int<5>
+ // CHECK: cir.scope
+ // CHECK: cir.if
+ // CHECK-NOT: cir.load
+ // CHECK: cir.call @use(%[[CONST]])
+ %0 = cir.alloca "a" align(4) : !cir.ptr<!s32i>
+ %1 = cir.const #cir.int<5> : !s32i
+ cir.store %1, %0 : !s32i, !cir.ptr<!s32i>
+ cir.scope {
+ cir.if %cond {
+ %2 = cir.load %0 : !cir.ptr<!s32i>, !s32i
+ cir.call @use(%2) : (!s32i) -> ()
+ cir.yield
+ }
+ }
+ cir.return
+ }
+
+ // CHECK-LABEL: cir.func @store_in_scope
+ cir.func @store_in_scope(%cond: !cir.bool) {
+ // CHECK: cir.alloca
+ // CHECK: cir.scope
+ // CHECK: cir.store
+ // CHECK: cir.load
+ %0 = cir.alloca "a" align(4) : !cir.ptr<!s32i>
+ %1 = cir.const #cir.int<1> : !s32i
+ cir.store %1, %0 : !s32i, !cir.ptr<!s32i>
+ cir.scope {
+ %2 = cir.const #cir.int<2> : !s32i
+ cir.store %2, %0 : !s32i, !cir.ptr<!s32i>
+ }
+ %3 = cir.load %0 : !cir.ptr<!s32i>, !s32i
+ cir.call @use(%3) : (!s32i) -> ()
+ cir.return
+ }
+
+ cir.func private @use(!s32i)
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/215780
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
