[clang] [CIR] Set memory effects of inline-asm-volatile correctly (PR #228261)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Erich Keane (erichkeane)

<details>
<summary>Changes</summary>

Inline-asm was set to have recursive memory effects, but it has no blocks, so the result was it never was listed as having memory effects.  This patch sets it to have a read and write effect in the case where it has side effects.

This showed up in the libcxx test, where we defeated the DoNotOptimze macro by not properly respecting inline-asm side effects.

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


3 Files Affected:

- (modified) clang/include/clang/CIR/Dialect/IR/CIROps.td (+2-1) 
- (modified) clang/lib/CIR/Dialect/IR/CIRDialect.cpp (+10) 
- (added) clang/test/CIR/Transforms/inline-asm-side-effects.cir (+28) 


``````````diff
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index bc9af2210a048e..6be840fbb343a3 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -5575,7 +5575,8 @@ def CIR_AsmFlavor : CIR_I32Enum<"AsmFlavor", "ATT or Intel",
 
 def CIR_AsmFlavorAttr : CIR_EnumAttr<CIR_AsmFlavor, "asm_flavor">;
 
-def CIR_InlineAsmOp : CIR_Op<"asm", [RecursiveMemoryEffects]> {
+def CIR_InlineAsmOp : CIR_Op<"asm",
+    [DeclareOpInterfaceMethods<MemoryEffectsOpInterface>]> {
   let description = [{
     The `cir.asm` operation represents C/C++ asm inline.
 
diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
index 4b9e0c7f0f544b..bba3e5ea94e8ec 100644
--- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
@@ -4672,6 +4672,16 @@ ParseResult cir::InlineAsmOp::parse(OpAsmParser &parser,
   return mlir::success();
 }
 
+void InlineAsmOp::getEffects(
+    llvm::SmallVectorImpl<mlir::MemoryEffects::EffectInstance> &effects) {
+  // If we have any side effects (that is, we're volatile asm), add a read and
+  // write memory effect. We do this the same as the llvm dialect InlineAsmOp.
+  if (getSideEffects()) {
+    effects.emplace_back(mlir::MemoryEffects::Read::get());
+    effects.emplace_back(mlir::MemoryEffects::Write::get());
+  }
+}
+
 //===----------------------------------------------------------------------===//
 // ThrowOp / TryThrowOp
 //===----------------------------------------------------------------------===//
diff --git a/clang/test/CIR/Transforms/inline-asm-side-effects.cir b/clang/test/CIR/Transforms/inline-asm-side-effects.cir
new file mode 100644
index 00000000000000..ddc8d1955e739e
--- /dev/null
+++ b/clang/test/CIR/Transforms/inline-asm-side-effects.cir
@@ -0,0 +1,28 @@
+// RUN: cir-opt %s -canonicalize -o - | FileCheck %s
+
+module {
+
+// CHECK-LABEL: @dead_pure_asm_is_removed
+// CHECK-NOT:     cir.asm
+cir.func @dead_pure_asm_is_removed() {
+  cir.asm(x86_att,
+    out = [],
+    in = [],
+    in_out = [],
+    {"nop" "~{dirflag},~{fpsr},~{flags}"})
+  cir.return
+}
+
+// CHECK-LABEL: @dead_side_effects_asm_is_not_removed
+// CHECK:         cir.asm
+// CHECK:         side_effects
+cir.func @dead_side_effects_asm_is_not_removed() {
+  cir.asm(x86_att,
+    out = [],
+    in = [],
+    in_out = [],
+    {"nop" "~{dirflag},~{fpsr},~{flags}"}) side_effects
+  cir.return
+}
+
+}

``````````

</details>


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