[clang] [clang][CodeGen] Emit improved memory effects and return status for AsmStmt (PR #110510)

Bruno De Fraine via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 30 06:41:06 PDT 2024


https://github.com/brunodf-snps created https://github.com/llvm/llvm-project/pull/110510

This patch adds an appropriate LLVM memory effects attribute and `willreturn` attribute to asm call instructions for extended asm statements. The existing code of EmitAsmStmt seems to have been written before the introduction of the new LLVM `memory` and `willreturn`/`mustprogress` attributes. It only considers `nounwind` and still targeted `readonly`/`readnone` attributes.

>From d3c93305b8626ac0ba6209ac7c83e511ad965ff3 Mon Sep 17 00:00:00 2001
From: Bruno De Fraine <brunodf at synopsys.com>
Date: Mon, 30 Sep 2024 15:12:51 +0200
Subject: [PATCH] [clang][CodeGen] Emit improved memory effects and return
 status for AsmStmt

---
 clang/lib/CodeGen/CGStmt.cpp              | 58 +++++++++++++----------
 clang/test/CodeGen/asm-attrs.c            | 18 +++----
 clang/test/CodeGen/mips-constraint-regs.c |  8 ++--
 clang/test/CodeGenCUDA/convergent.cu      |  4 +-
 4 files changed, 48 insertions(+), 40 deletions(-)

diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index 9bf15fca0de489..210cef68506f3a 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -2473,9 +2473,9 @@ static llvm::MDNode *getAsmSrcLocInfo(const StringLiteral *Str,
 }
 
 static void UpdateAsmCallInst(llvm::CallBase &Result, bool HasSideEffect,
-                              bool HasUnwindClobber, bool ReadOnly,
-                              bool ReadNone, bool NoMerge, bool NoConvergent,
-                              const AsmStmt &S,
+                              bool HasUnwindClobber,
+                              llvm::MemoryEffects MemoryEffects, bool NoMerge,
+                              bool NoConvergent, const AsmStmt &S,
                               const std::vector<llvm::Type *> &ResultRegTypes,
                               const std::vector<llvm::Type *> &ArgElemTypes,
                               CodeGenFunction &CGF,
@@ -2483,15 +2483,17 @@ static void UpdateAsmCallInst(llvm::CallBase &Result, bool HasSideEffect,
   if (!HasUnwindClobber)
     Result.addFnAttr(llvm::Attribute::NoUnwind);
 
+  // Assume inline asm will return unless there is a sideeffect (not listed in
+  // the constraints)
+  if (!HasSideEffect)
+    Result.addFnAttr(llvm::Attribute::WillReturn);
+
   if (NoMerge)
     Result.addFnAttr(llvm::Attribute::NoMerge);
-  // Attach readnone and readonly attributes.
-  if (!HasSideEffect) {
-    if (ReadNone)
-      Result.setDoesNotAccessMemory();
-    else if (ReadOnly)
-      Result.setOnlyReadsMemory();
-  }
+
+  // Attach memory effects when known.
+  if (MemoryEffects != llvm::MemoryEffects::unknown())
+    Result.setMemoryEffects(MemoryEffects);
 
   // Add elementtype attribute for indirect constraints.
   for (auto Pair : llvm::enumerate(ArgElemTypes)) {
@@ -2704,13 +2706,19 @@ void CodeGenFunction::EmitAsmStmt(const AsmStmt &S) {
   // Keep track of defined physregs.
   llvm::SmallSet<std::string, 8> PhysRegOutputs;
 
-  // An inline asm can be marked readonly if it meets the following conditions:
-  //  - it doesn't have any sideeffects
-  //  - it doesn't clobber memory
-  //  - it doesn't return a value by-reference
-  // It can be marked readnone if it doesn't have any input memory constraints
-  // in addition to meeting the conditions listed above.
-  bool ReadOnly = true, ReadNone = true;
+  // An inline asm is implicitly volatile if it has no ouputs (including simple
+  // asm)
+  bool HasSideEffect = S.isVolatile() || S.getNumOutputs() == 0;
+
+  // Conservatively assume simple (basic) asm has unknown memory access. For
+  // extended asm,
+  //  - add inaccessiblemem if it has sideeffects
+  //  - add argmem read/write for input/output operands with memory constraints
+  //  - fall back to unknown memory access when it clobbers memory
+  llvm::MemoryEffects MemoryEffects =
+      S.isSimple() ? llvm::MemoryEffects::unknown()
+                   : (HasSideEffect ? llvm::MemoryEffects::inaccessibleMemOnly()
+                                    : llvm::MemoryEffects::none());
 
   for (unsigned i = 0, e = S.getNumOutputs(); i != e; i++) {
     TargetInfo::ConstraintInfo &Info = OutputConstraintInfos[i];
@@ -2818,7 +2826,7 @@ void CodeGenFunction::EmitAsmStmt(const AsmStmt &S) {
       Args.push_back(DestAddr.emitRawPointer(*this));
       Constraints += "=*";
       Constraints += OutputConstraint;
-      ReadOnly = ReadNone = false;
+      MemoryEffects |= llvm::MemoryEffects::argMemOnly(llvm::ModRefInfo::Mod);
     }
 
     if (Info.isReadWrite()) {
@@ -2873,7 +2881,7 @@ void CodeGenFunction::EmitAsmStmt(const AsmStmt &S) {
     TargetInfo::ConstraintInfo &Info = InputConstraintInfos[i];
 
     if (Info.allowsMemory())
-      ReadNone = false;
+      MemoryEffects |= llvm::MemoryEffects::argMemOnly(llvm::ModRefInfo::Ref);
 
     if (!Constraints.empty())
       Constraints += ',';
@@ -2971,7 +2979,7 @@ void CodeGenFunction::EmitAsmStmt(const AsmStmt &S) {
     StringRef Clobber = S.getClobber(i);
 
     if (Clobber == "memory")
-      ReadOnly = ReadNone = false;
+      MemoryEffects = llvm::MemoryEffects::unknown();
     else if (Clobber == "unwind") {
       HasUnwindClobber = true;
       continue;
@@ -3031,8 +3039,6 @@ void CodeGenFunction::EmitAsmStmt(const AsmStmt &S) {
   llvm::FunctionType *FTy =
     llvm::FunctionType::get(ResultType, ArgTypes, false);
 
-  bool HasSideEffect = S.isVolatile() || S.getNumOutputs() == 0;
-
   llvm::InlineAsm::AsmDialect GnuAsmDialect =
       CGM.getCodeGenOpts().getInlineAsmDialect() == CodeGenOptions::IAD_ATT
           ? llvm::InlineAsm::AD_ATT
@@ -3050,8 +3056,8 @@ void CodeGenFunction::EmitAsmStmt(const AsmStmt &S) {
   if (IsGCCAsmGoto) {
     CBR = Builder.CreateCallBr(IA, Fallthrough, Transfer, Args);
     EmitBlock(Fallthrough);
-    UpdateAsmCallInst(*CBR, HasSideEffect, /*HasUnwindClobber=*/false, ReadOnly,
-                      ReadNone, InNoMergeAttributedStmt,
+    UpdateAsmCallInst(*CBR, HasSideEffect, /*HasUnwindClobber=*/false,
+                      MemoryEffects, InNoMergeAttributedStmt,
                       InNoConvergentAttributedStmt, S, ResultRegTypes,
                       ArgElemTypes, *this, RegResults);
     // Because we are emitting code top to bottom, we don't have enough
@@ -3082,14 +3088,14 @@ void CodeGenFunction::EmitAsmStmt(const AsmStmt &S) {
   } else if (HasUnwindClobber) {
     llvm::CallBase *Result = EmitCallOrInvoke(IA, Args, "");
     UpdateAsmCallInst(*Result, HasSideEffect, /*HasUnwindClobber=*/true,
-                      ReadOnly, ReadNone, InNoMergeAttributedStmt,
+                      MemoryEffects, InNoMergeAttributedStmt,
                       InNoConvergentAttributedStmt, S, ResultRegTypes,
                       ArgElemTypes, *this, RegResults);
   } else {
     llvm::CallInst *Result =
         Builder.CreateCall(IA, Args, getBundlesForFunclet(IA));
     UpdateAsmCallInst(*Result, HasSideEffect, /*HasUnwindClobber=*/false,
-                      ReadOnly, ReadNone, InNoMergeAttributedStmt,
+                      MemoryEffects, InNoMergeAttributedStmt,
                       InNoConvergentAttributedStmt, S, ResultRegTypes,
                       ArgElemTypes, *this, RegResults);
   }
diff --git a/clang/test/CodeGen/asm-attrs.c b/clang/test/CodeGen/asm-attrs.c
index 6d95e10d0af0b2..342f2cf6d464a3 100644
--- a/clang/test/CodeGen/asm-attrs.c
+++ b/clang/test/CodeGen/asm-attrs.c
@@ -3,16 +3,18 @@
 // CHECK: call i32 asm "foo0", {{.*}} [[READNONE:#[0-9]+]]
 // CHECK: call i32 asm "foo1", {{.*}} [[READNONE]]
 // CHECK: call i32 asm "foo2", {{.*}} [[NOATTRS:#[0-9]+]]
-// CHECK: call i32 asm sideeffect "foo3", {{.*}} [[NOATTRS]]
-// CHECK: call i32 asm "foo4", {{.*}} [[READONLY:#[0-9]+]]
-// CHECK: call i32 asm "foo5", {{.*}} [[READONLY]]
-// CHECK: call i32 asm "foo6", {{.*}} [[NOATTRS]]
-// CHECK: call void asm sideeffect "foo7", {{.*}} [[NOATTRS]]
+// CHECK: call i32 asm sideeffect "foo3", {{.*}} [[INACCESSIBLEMEMONLY:#[0-9]+]]
+// CHECK: call i32 asm "foo4", {{.*}} [[ARGREAD:#[0-9]+]]
+// CHECK: call i32 asm "foo5", {{.*}} [[ARGREAD]]
+// CHECK: call i32 asm "foo6", {{.*}} [[ARGWRITE:#[0-9]+]]
+// CHECK: call void asm sideeffect "foo7", {{.*}} [[INACCESSIBLEMEMONLY]]
 // CHECK: call i32 asm "foo8", {{.*}} [[READNONE]]
 
-// CHECK: attributes [[READNONE]] = { nounwind memory(none) }
-// CHECK: attributes [[NOATTRS]] = { nounwind }
-// CHECK: attributes [[READONLY]] = { nounwind memory(read) }
+// CHECK: attributes [[READNONE]] = { nounwind willreturn memory(none) }
+// CHECK: attributes [[NOATTRS]] = { nounwind willreturn }
+// CHECK: attributes [[INACCESSIBLEMEMONLY]] = { nounwind memory(inaccessiblemem: readwrite) }
+// CHECK: attributes [[ARGREAD]] = { nounwind willreturn memory(argmem: read) }
+// CHECK: attributes [[ARGWRITE]] = { nounwind willreturn memory(argmem: write) }
 
 int g0, g1;
 
diff --git a/clang/test/CodeGen/mips-constraint-regs.c b/clang/test/CodeGen/mips-constraint-regs.c
index f6ee2a17f0abff..2c06ca2d21645c 100644
--- a/clang/test/CodeGen/mips-constraint-regs.c
+++ b/clang/test/CodeGen/mips-constraint-regs.c
@@ -9,7 +9,7 @@ int main(void)
   // 'c': 16 bit address register for Mips16, GPR for all others
   // I am using 'c' to constrain both the target and one of the source
   // registers. We are looking for syntactical correctness.
-  // CHECK: %{{[0-9]+}} = call i32 asm sideeffect "addi $0,$1,$2 \0A\09\09", "=c,c,I,~{$1}"(i32 %{{[0-9]+}}, i32 %{{[0-9]+}}) [[NUW:#[0-9]+]], !srcloc !{{[0-9]+}}
+  // CHECK: %{{[0-9]+}} = call i32 asm sideeffect "addi $0,$1,$2 \0A\09\09", "=c,c,I,~{$1}"(i32 %{{[0-9]+}}, i32 %{{[0-9]+}}) [[ATTR:#[0-9]+]], !srcloc !{{[0-9]+}}
   int __s, __v = 17;
   int __t;
   __asm__ __volatile__(
@@ -20,7 +20,7 @@ int main(void)
   // 'l': lo register
   // We are making it clear that destination register is lo with the
   // use of the 'l' constraint ("=l").
-  // CHECK:   %{{[0-9]+}} = call i32 asm sideeffect "mtlo $1 \0A\09\09", "=l,r,~{lo},~{$1}"(i32 %{{[0-9]+}}) [[NUW]], !srcloc !{{[0-9]+}}
+  // CHECK:   %{{[0-9]+}} = call i32 asm sideeffect "mtlo $1 \0A\09\09", "=l,r,~{lo},~{$1}"(i32 %{{[0-9]+}}) [[ATTR]], !srcloc !{{[0-9]+}}
   int i_temp = 44;
   int i_result;
   __asm__ __volatile__(
@@ -32,7 +32,7 @@ int main(void)
   // 'x': Combined lo/hi registers
   // We are specifying that destination registers are the hi/lo pair with the
   // use of the 'x' constraint ("=x").
-  // CHECK:  %{{[0-9]+}} = call i64 asm sideeffect "mthi $1 \0A\09\09mtlo $2 \0A\09\09", "=x,r,r,~{$1}"(i32 %{{[0-9]+}}, i32 %{{[0-9]+}}) [[NUW]], !srcloc !{{[0-9]+}}
+  // CHECK:  %{{[0-9]+}} = call i64 asm sideeffect "mthi $1 \0A\09\09mtlo $2 \0A\09\09", "=x,r,r,~{$1}"(i32 %{{[0-9]+}}, i32 %{{[0-9]+}}) [[ATTR]], !srcloc !{{[0-9]+}}
   int i_hi = 3;
   int i_lo = 2;
   long long ll_result = 0;
@@ -46,4 +46,4 @@ int main(void)
   return 0;
 }
 
-// CHECK: attributes [[NUW]] = { nounwind }
+// CHECK: attributes [[ATTR]] = { nounwind memory(inaccessiblemem: readwrite) }
diff --git a/clang/test/CodeGenCUDA/convergent.cu b/clang/test/CodeGenCUDA/convergent.cu
index b187f3a8a32d69..e8a61eee29ab4c 100644
--- a/clang/test/CodeGenCUDA/convergent.cu
+++ b/clang/test/CodeGenCUDA/convergent.cu
@@ -76,12 +76,12 @@ __host__ __device__ void bar() {
 // DEVICE: attributes #[[ATTR2:[0-9]+]] = { convergent nounwind "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+ptx32" }
 // DEVICE: attributes #[[ATTR3:[0-9]+]] = { nounwind "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+ptx32" }
 // DEVICE: attributes #[[ATTR4]] = { convergent nounwind }
-// DEVICE: attributes #[[ATTR5]] = { convergent nounwind memory(none) }
+// DEVICE: attributes #[[ATTR5]] = { convergent nounwind willreturn memory(none) }
 // DEVICE: attributes #[[ATTR6]] = { nounwind }
 //.
 // HOST: attributes #[[ATTR0]] = { mustprogress noinline nounwind optnone "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
 // HOST: attributes #[[ATTR1:[0-9]+]] = { "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
-// HOST: attributes #[[ATTR2]] = { nounwind memory(none) }
+// HOST: attributes #[[ATTR2]] = { nounwind willreturn memory(none) }
 // HOST: attributes #[[ATTR3]] = { nounwind }
 //.
 // DEVICE: [[META0:![0-9]+]] = !{i32 1, !"wchar_size", i32 4}



More information about the cfe-commits mailing list