https://github.com/Lancern created https://github.com/llvm/llvm-project/pull/212262
Current mem2reg implementation for `cir.load` and `cir.store` does not check whether the load/store is volatile or atomic. Fix this. >From 35d23ed82794da7b0709db9af3d45e88b2dd11a8 Mon Sep 17 00:00:00 2001 From: Sirui Mu <[email protected]> Date: Mon, 27 Jul 2026 22:36:51 +0800 Subject: [PATCH] [CIR] Disallow mem2reg for volatile/atomic loads and stores Current mem2reg implementation for `cir.load` and `cir.store` does not check whether the load/store is volatile or atomic. Fix this. --- clang/lib/CIR/Dialect/IR/CIRMemorySlot.cpp | 14 +++++- clang/test/CIR/Transforms/mem2reg.cir | 55 ++++++++++++++++++++-- 2 files changed, 63 insertions(+), 6 deletions(-) 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 + } } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
