[clang] [CIR] Disallow mem2reg for volatile/atomic loads and stores (PR #212262)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clangir

Author: Sirui Mu (Lancern)

<details>
<summary>Changes</summary>

Current mem2reg implementation for `cir.load` and `cir.store` does not check whether the load/store is volatile or atomic. Fix this.

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


2 Files Affected:

- (modified) clang/lib/CIR/Dialect/IR/CIRMemorySlot.cpp (+12-2) 
- (modified) clang/test/CIR/Transforms/mem2reg.cir (+51-4) 


``````````diff
diff --git a/clang/lib/CIR/Dialect/IR/CIRMemorySlot.cpp b/clang/lib/CIR/Dialect/IR/CIRMemorySlot.cpp
index 73b35c7f00c2d..d6de6b6e80799 100644
--- a/clang/lib/CIR/Dialect/IR/CIRMemorySlot.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRMemorySlot.cpp
@@ -72,6 +72,11 @@ bool cir::LoadOp::canUsesBeRemoved(
     const DataLayout &dataLayout) {
   if (blockingUses.size() != 1)
     return false;
+
+  // Volatile load or atomic load should not be removed.
+  if (getIsVolatile() || getMemOrder().has_value())
+    return false;
+
   Value blockingUse = (*blockingUses.begin())->get();
   return blockingUse == slot.ptr && getAddr() == slot.ptr &&
          getType() == slot.elemType;
@@ -106,6 +111,11 @@ bool cir::StoreOp::canUsesBeRemoved(
     const DataLayout &dataLayout) {
   if (blockingUses.size() != 1)
     return false;
+
+  // Volatile store or atomic store should not be removed.
+  if (getIsVolatile() || getMemOrder().has_value())
+    return false;
+
   Value blockingUse = (*blockingUses.begin())->get();
   return blockingUse == slot.ptr && getAddr() == slot.ptr &&
          getValue() != slot.ptr && slot.elemType == getValue().getType();
@@ -141,8 +151,8 @@ DeletionKind cir::CopyOp::removeBlockingUses(
     const DataLayout &dataLayout) {
   if (loadsFrom(slot))
     cir::StoreOp::create(builder, getLoc(), reachingDefinition, getDst(),
-                         /*isVolatile=*/false,
-                         /*isNontemporal=*/false,
+                         /*is_volatile=*/false,
+                         /*is_nontemporal=*/false,
                          /*alignment=*/mlir::IntegerAttr{},
                          /*sync_scope=*/cir::SyncScopeKindAttr(),
                          /*mem-order=*/cir::MemOrderAttr());
diff --git a/clang/test/CIR/Transforms/mem2reg.cir b/clang/test/CIR/Transforms/mem2reg.cir
index 2319de275700c..84f83595f1c6c 100644
--- a/clang/test/CIR/Transforms/mem2reg.cir
+++ b/clang/test/CIR/Transforms/mem2reg.cir
@@ -1,7 +1,4 @@
-// RUN: cir-opt %s -cir-flatten-cfg -mem2reg -o - | FileCheck %s \
-// RUN:   --implicit-check-not=cir.alloca \
-// RUN:   --implicit-check-not=cir.load \
-// RUN:   --implicit-check-not=cir.store
+// RUN: cir-opt %s -cir-flatten-cfg -mem2reg -o - | FileCheck %s
 
 !s32i = !cir.int<s, 32>
 
@@ -20,4 +17,54 @@ module {
     %4 = cir.load %0 : !cir.ptr<!s32i>, !s32i
     cir.return %4 : !s32i
   }
+
+  // CHECK-LABEL: cir.func @do_not_promote_volatile
+  cir.func @do_not_promote_volatile_load() -> !s32i {
+    // CHECK: %[[ALLOCA:.*]] = cir.alloca "y" align(4) init : !cir.ptr<!s32i>
+    // CHECK: %[[RET:.+]] = cir.load volatile %[[ALLOCA]] : !cir.ptr<!s32i>, !s32i
+    // CHECK: cir.return %[[RET]] : !s32i
+    %0 = cir.alloca "y" align(4) init : !cir.ptr<!s32i>
+    %1 = cir.const #cir.int<42> : !s32i
+    cir.store %1, %0 : !s32i, !cir.ptr<!s32i>
+    %2 = cir.load volatile %0 : !cir.ptr<!s32i>, !s32i
+    cir.return %2 : !s32i
+  }
+
+  // CHECK-LABEL: cir.func @do_not_promote_volatile_store
+  cir.func @do_not_promote_volatile_store() -> !s32i {
+    // CHECK: %[[ALLOCA:.*]] = cir.alloca "y" align(4) init : !cir.ptr<!s32i>
+    // CHECK: cir.store volatile %{{.+}}, %[[ALLOCA]] : !s32i, !cir.ptr<!s32i>
+    // CHECK: %[[RET:.+]] = cir.load %[[ALLOCA]] : !cir.ptr<!s32i>, !s32i
+    // CHECK: cir.return %[[RET]] : !s32i
+    %0 = cir.alloca "y" align(4) init : !cir.ptr<!s32i>
+    %1 = cir.const #cir.int<42> : !s32i
+    cir.store volatile %1, %0 : !s32i, !cir.ptr<!s32i>
+    %2 = cir.load %0 : !cir.ptr<!s32i>, !s32i
+    cir.return %2 : !s32i
+  }
+
+  // CHECK-LABEL: cir.func @do_not_promote_atomic_load
+  cir.func @do_not_promote_atomic_load() -> !s32i {
+    // CHECK: %[[ALLOCA:.*]] = cir.alloca "y" align(4) init : !cir.ptr<!s32i>
+    // CHECK: %[[RET:.+]] = cir.load atomic(acquire) %[[ALLOCA]] : !cir.ptr<!s32i>, !s32i
+    // CHECK: cir.return %[[RET]] : !s32i
+    %0 = cir.alloca "y" align(4) init : !cir.ptr<!s32i>
+    %1 = cir.const #cir.int<42> : !s32i
+    cir.store %1, %0 : !s32i, !cir.ptr<!s32i>
+    %2 = cir.load atomic(acquire) %0 : !cir.ptr<!s32i>, !s32i
+    cir.return %2 : !s32i
+  }
+
+  // CHECK-LABEL: cir.func @do_not_promote_atomic_store
+  cir.func @do_not_promote_atomic_store() -> !s32i {
+    // CHECK: %[[ALLOCA:.*]] = cir.alloca "y" align(4) init : !cir.ptr<!s32i>
+    // CHECK: cir.store atomic(release) %{{.+}}, %[[ALLOCA]] : !s32i, !cir.ptr<!s32i>
+    // CHECK: %[[RET:.+]] = cir.load %[[ALLOCA]] : !cir.ptr<!s32i>, !s32i
+    // CHECK: cir.return %[[RET]] : !s32i
+    %0 = cir.alloca "y" align(4) init : !cir.ptr<!s32i>
+    %1 = cir.const #cir.int<42> : !s32i
+    cir.store atomic(release) %1, %0 : !s32i, !cir.ptr<!s32i>
+    %2 = cir.load %0 : !cir.ptr<!s32i>, !s32i
+    cir.return %2 : !s32i
+  }
 }

``````````

</details>


https://github.com/llvm/llvm-project/pull/212262