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 <brun...@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} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits