https://github.com/isanbard updated https://github.com/llvm/llvm-project/pull/236708
>From a32d772433319cb2c43f9ff55b636250af8a059d Mon Sep 17 00:00:00 2001 From: Bill Wendling <[email protected]> Date: Thu, 29 Oct 2026 02:32:09 -0700 Subject: [PATCH 1/5] [Clang][CodeGen] Update the TBAA info for potential aliases The stores of a _Complex number were tagged as "unsigned long", but loads were tagged as "float". TBAA reports no aliases if the store and loads went through different pointers and GVN forwards a stale value. Fix this by giving the "store" the "omnipotent char" tag, so that TBAA treats it as aliasing every access to the destination's members. Using that tag over "Dest.getTBAAInfo()" because it doesn't depend on how CodeGenTBAA happens to tag struct and _Complex types. Assisted-by: Claude for testcases. --- clang/lib/CodeGen/CGStmt.cpp | 6 +++- .../test/CodeGen/X86/inline-asm-output-tbaa.c | 28 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 clang/test/CodeGen/X86/inline-asm-output-tbaa.c diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp index 05737ce071647d..dc34bfc0f5e2c6 100644 --- a/clang/lib/CodeGen/CGStmt.cpp +++ b/clang/lib/CodeGen/CGStmt.cpp @@ -3389,7 +3389,11 @@ void AsmConstraintsInfo::EmitAsmStores( return; } - Dest = CGF.MakeAddrLValue(A, Ty); + // The integer isn't the destination's type, so a TBAA tag for it would + // claim that the store doesn't alias accesses to the destination's + // members. + Dest = CGF.MakeAddrLValue(A, Ty, Dest.getBaseInfo(), + TBAAAccessInfo::getMayAliasInfo()); } CGF.EmitStoreThroughLValue(RValue::get(Tmp), Dest); diff --git a/clang/test/CodeGen/X86/inline-asm-output-tbaa.c b/clang/test/CodeGen/X86/inline-asm-output-tbaa.c new file mode 100644 index 00000000000000..90cb24d0c899e6 --- /dev/null +++ b/clang/test/CodeGen/X86/inline-asm-output-tbaa.c @@ -0,0 +1,28 @@ +// REQUIRES: x86-registered-target +// Clang emits TBAA only when optimizing, so this test needs -O1. +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -O1 -disable-llvm-passes -emit-llvm %s -o - | FileCheck %s + +// A struct or _Complex output with a register constraint is returned as an +// integer and stored to the destination. The store must not have the integer's +// TBAA tag, or it wouldn't alias accesses to the destination's members. + +struct S { + float a, b; +}; + +// CHECK-LABEL: define {{.*}}void @test_struct_output( +// CHECK: [[RES:%.*]] = call i64 asm "", "=r, +// CHECK-NEXT: store i64 [[RES]], ptr %{{.*}}, align 4, !tbaa [[CHAR_TAG:![0-9]+]] +void test_struct_output(struct S *p) { + asm("" : "=r"(*p)); +} + +// CHECK-LABEL: define {{.*}}void @test_complex_output( +// CHECK: [[RES:%.*]] = call i64 asm "", "=r, +// CHECK-NEXT: store i64 [[RES]], ptr %{{.*}}, align 4, !tbaa [[CHAR_TAG]] +void test_complex_output(_Complex float *p) { + asm("" : "=r"(*p)); +} + +// CHECK-DAG: [[CHAR_TAG]] = !{[[CHAR:![0-9]+]], [[CHAR]], i64 0} +// CHECK-DAG: [[CHAR]] = !{!"omnipotent char", >From 903e92e96e60b525401877d499bcbd4810f5057c Mon Sep 17 00:00:00 2001 From: Bill Wendling <[email protected]> Date: Thu, 29 Oct 2026 02:50:48 -0700 Subject: [PATCH 2/5] Remove the _Complex case from the TBAA test _Complex outputs aren't returned by value until #236695. Assisted-by: Claude Opus 5.5 --- clang/test/CodeGen/X86/inline-asm-output-tbaa.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/clang/test/CodeGen/X86/inline-asm-output-tbaa.c b/clang/test/CodeGen/X86/inline-asm-output-tbaa.c index 90cb24d0c899e6..9c6ab17d6818e0 100644 --- a/clang/test/CodeGen/X86/inline-asm-output-tbaa.c +++ b/clang/test/CodeGen/X86/inline-asm-output-tbaa.c @@ -2,9 +2,9 @@ // Clang emits TBAA only when optimizing, so this test needs -O1. // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -O1 -disable-llvm-passes -emit-llvm %s -o - | FileCheck %s -// A struct or _Complex output with a register constraint is returned as an -// integer and stored to the destination. The store must not have the integer's -// TBAA tag, or it wouldn't alias accesses to the destination's members. +// A struct output with a register constraint is returned as an integer and +// stored to the destination. The store must not have the integer's TBAA tag, +// or it wouldn't alias accesses to the destination's members. struct S { float a, b; @@ -17,12 +17,5 @@ void test_struct_output(struct S *p) { asm("" : "=r"(*p)); } -// CHECK-LABEL: define {{.*}}void @test_complex_output( -// CHECK: [[RES:%.*]] = call i64 asm "", "=r, -// CHECK-NEXT: store i64 [[RES]], ptr %{{.*}}, align 4, !tbaa [[CHAR_TAG]] -void test_complex_output(_Complex float *p) { - asm("" : "=r"(*p)); -} - // CHECK-DAG: [[CHAR_TAG]] = !{[[CHAR:![0-9]+]], [[CHAR]], i64 0} // CHECK-DAG: [[CHAR]] = !{!"omnipotent char", >From 848381b1c7f69694ce3950a27149328a820f90d8 Mon Sep 17 00:00:00 2001 From: Bill Wendling <[email protected]> Date: Thu, 29 Oct 2026 02:55:59 -0700 Subject: [PATCH 3/5] [Clang][CodeGen] Fix volatile and size of inline asm struct output stores There are two issues fixed by this PR: one where 'volatile' is dropped and the other where an output tied to a larger input is stored out-of-bounds: - Struct outputs returned as integers are stored through an unqualified integer lvalue. Two write to a volatile struct collapse into one non-volatile store at -O2. - A tied input larger than the output makes the asm return the input's type, and the whole value is stored, resulting in an out-of-bounds write. Fix these by propagating the 'volatile' qualifier correctly and storing only the destination's bits instead of relying upon the input's size. Assisted-by: Claude Opus 5.5 --- clang/lib/CodeGen/CGStmt.cpp | 19 ++++- clang/test/CodeGen/AArch64/ls64-inline-asm.c | 13 +++ .../CodeGen/X86/inline-asm-output-store.c | 84 +++++++++++++++++++ 3 files changed, 112 insertions(+), 4 deletions(-) create mode 100644 clang/test/CodeGen/X86/inline-asm-output-store.c diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp index dc34bfc0f5e2c6..8255a078341f83 100644 --- a/clang/lib/CodeGen/CGStmt.cpp +++ b/clang/lib/CodeGen/CGStmt.cpp @@ -3376,7 +3376,8 @@ void AsmConstraintsInfo::EmitAsmStores( Address A = Dest.getAddress().withElementType(ResultRegTypes[i]); if (getTargetHooks().isScalarizableAsmOperand(CGF, TruncTy)) { - llvm::StoreInst *S = Builder.CreateStore(Tmp, A); + llvm::StoreInst *S = + Builder.CreateStore(Tmp, A, Dest.isVolatileQualified()); CGF.addInstToCurrentSourceAtom(S, S->getValueOperand()); continue; } @@ -3389,11 +3390,21 @@ void AsmConstraintsInfo::EmitAsmStores( return; } + // An output tied to a larger input has the input's type, so store only + // the destination's bits. + llvm::Type *IntTy = llvm::IntegerType::get(CTX, Size); + if (Tmp->getType()->isPointerTy()) + Tmp = Builder.CreatePtrToInt(Tmp, IntTy); + else + Tmp = Builder.CreateTrunc(Tmp, IntTy); + // The integer isn't the destination's type, so a TBAA tag for it would // claim that the store doesn't alias accesses to the destination's - // members. - Dest = CGF.MakeAddrLValue(A, Ty, Dest.getBaseInfo(), - TBAAAccessInfo::getMayAliasInfo()); + // members. Keep the destination's qualifiers, such as volatile. + Dest = CGF.MakeAddrLValue( + Dest.getAddress().withElementType(IntTy), + getContext().getQualifiedType(Ty, Dest.getQuals()), + Dest.getBaseInfo(), TBAAAccessInfo::getMayAliasInfo()); } CGF.EmitStoreThroughLValue(RValue::get(Tmp), Dest); diff --git a/clang/test/CodeGen/AArch64/ls64-inline-asm.c b/clang/test/CodeGen/AArch64/ls64-inline-asm.c index 6290ee5595f942..f3279b5c32ff4a 100644 --- a/clang/test/CodeGen/AArch64/ls64-inline-asm.c +++ b/clang/test/CodeGen/AArch64/ls64-inline-asm.c @@ -83,6 +83,18 @@ void store2(int *in, void *addr) struct foo s = { in[0], in[1], in[4], in[16], in[25], in[36], in[49], in[64] }; __asm__ volatile ("st64b %0,[%1]" : : "r" (s), "r" (addr) : "memory" ); } + +// CHECK-LABEL: define dso_local void @load_volatile( +// CHECK-SAME: ptr nofree noundef captures(address) [[OUTPUT:%.*]], ptr noundef [[ADDR:%.*]]) local_unnamed_addr #[[ATTR0]] { +// CHECK-NEXT: [[ENTRY:.*:]] +// CHECK-NEXT: [[TMP0:%.*]] = tail call i512 asm sideeffect "ld64b $0,[$1]", "=r,r,~{memory}"(ptr [[ADDR]]) #[[ATTR1]], !srcloc [[META13:![0-9]+]] +// CHECK-NEXT: store volatile i512 [[TMP0]], ptr [[OUTPUT]], align 8 +// CHECK-NEXT: ret void +// +void load_volatile(volatile struct foo *output, void *addr) +{ + __asm__ volatile ("ld64b %0,[%1]" : "=r" (*output) : "r" (addr) : "memory"); +} //. // CHECK: [[META3:![0-9]+]] = !{!"int", [[META4:![0-9]+]], i64 0} // CHECK: [[META4]] = !{!"omnipotent char", [[META5:![0-9]+]], i64 0} @@ -91,4 +103,5 @@ void store2(int *in, void *addr) // CHECK: [[META7]] = !{i64 1382} // CHECK: [[INT_TBAA8]] = !{[[META3]], [[META3]], i64 0} // CHECK: [[META9]] = !{i64 6013} +// CHECK: [[META13]] = !{i64 6616} //. diff --git a/clang/test/CodeGen/X86/inline-asm-output-store.c b/clang/test/CodeGen/X86/inline-asm-output-store.c new file mode 100644 index 00000000000000..819067b2e0f5e3 --- /dev/null +++ b/clang/test/CodeGen/X86/inline-asm-output-store.c @@ -0,0 +1,84 @@ +// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --check-globals none --version 6 +// REQUIRES: x86-registered-target +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o - | FileCheck %s + +// A struct output with a register constraint is returned as an integer and +// stored to the destination. The store keeps the destination's volatility, and +// it writes only the destination's bits when the output is tied to a larger +// input, which makes the asm return the input's type. + +struct C { + char a, b; +}; + +struct S { + float a, b; +}; + +// CHECK-LABEL: define dso_local void @test_volatile( +// CHECK-SAME: ptr noundef [[P:%.*]]) #[[ATTR0:[0-9]+]] { +// CHECK-NEXT: [[ENTRY:.*:]] +// CHECK-NEXT: [[P_ADDR:%.*]] = alloca ptr, align 8 +// CHECK-NEXT: store ptr [[P]], ptr [[P_ADDR]], align 8 +// CHECK-NEXT: [[TMP0:%.*]] = load ptr, ptr [[P_ADDR]], align 8 +// CHECK-NEXT: [[TMP1:%.*]] = call i64 asm "", "=r,~{dirflag},~{fpsr},~{flags}"() #[[ATTR1:[0-9]+]], !srcloc [[META1:![0-9]+]] +// CHECK-NEXT: store volatile i64 [[TMP1]], ptr [[TMP0]], align 4 +// CHECK-NEXT: ret void +// +void test_volatile(volatile struct S *p) { + asm("" : "=r"(*p)); +} + +// CHECK-LABEL: define dso_local void @test_tied_int( +// CHECK-SAME: ptr noundef [[P:%.*]], i32 noundef [[X:%.*]]) #[[ATTR0]] { +// CHECK-NEXT: [[ENTRY:.*:]] +// CHECK-NEXT: [[P_ADDR:%.*]] = alloca ptr, align 8 +// CHECK-NEXT: [[X_ADDR:%.*]] = alloca i32, align 4 +// CHECK-NEXT: store ptr [[P]], ptr [[P_ADDR]], align 8 +// CHECK-NEXT: store i32 [[X]], ptr [[X_ADDR]], align 4 +// CHECK-NEXT: [[TMP0:%.*]] = load ptr, ptr [[P_ADDR]], align 8 +// CHECK-NEXT: [[TMP1:%.*]] = load i32, ptr [[X_ADDR]], align 4 +// CHECK-NEXT: [[TMP2:%.*]] = call i32 asm "", "=r,0,~{dirflag},~{fpsr},~{flags}"(i32 [[TMP1]]) #[[ATTR1]], !srcloc [[META2:![0-9]+]] +// CHECK-NEXT: [[TMP3:%.*]] = trunc i32 [[TMP2]] to i16 +// CHECK-NEXT: store i16 [[TMP3]], ptr [[TMP0]], align 1 +// CHECK-NEXT: ret void +// +void test_tied_int(struct C *p, int x) { + asm("" : "=r"(*p) : "0"(x)); +} + +// CHECK-LABEL: define dso_local void @test_tied_pointer( +// CHECK-SAME: ptr noundef [[P:%.*]], ptr noundef [[X:%.*]]) #[[ATTR0]] { +// CHECK-NEXT: [[ENTRY:.*:]] +// CHECK-NEXT: [[P_ADDR:%.*]] = alloca ptr, align 8 +// CHECK-NEXT: [[X_ADDR:%.*]] = alloca ptr, align 8 +// CHECK-NEXT: store ptr [[P]], ptr [[P_ADDR]], align 8 +// CHECK-NEXT: store ptr [[X]], ptr [[X_ADDR]], align 8 +// CHECK-NEXT: [[TMP0:%.*]] = load ptr, ptr [[P_ADDR]], align 8 +// CHECK-NEXT: [[TMP1:%.*]] = load ptr, ptr [[X_ADDR]], align 8 +// CHECK-NEXT: [[TMP2:%.*]] = call ptr asm "", "=r,0,~{dirflag},~{fpsr},~{flags}"(ptr [[TMP1]]) #[[ATTR1]], !srcloc [[META3:![0-9]+]] +// CHECK-NEXT: [[TMP3:%.*]] = ptrtoint ptr [[TMP2]] to i16 +// CHECK-NEXT: store i16 [[TMP3]], ptr [[TMP0]], align 1 +// CHECK-NEXT: ret void +// +void test_tied_pointer(struct C *p, void *x) { + asm("" : "=r"(*p) : "0"(x)); +} + +// The input is the same size, so the asm returns the destination's size. +// CHECK-LABEL: define dso_local void @test_tied_same_size( +// CHECK-SAME: ptr noundef [[P:%.*]], i16 noundef signext [[X:%.*]]) #[[ATTR0]] { +// CHECK-NEXT: [[ENTRY:.*:]] +// CHECK-NEXT: [[P_ADDR:%.*]] = alloca ptr, align 8 +// CHECK-NEXT: [[X_ADDR:%.*]] = alloca i16, align 2 +// CHECK-NEXT: store ptr [[P]], ptr [[P_ADDR]], align 8 +// CHECK-NEXT: store i16 [[X]], ptr [[X_ADDR]], align 2 +// CHECK-NEXT: [[TMP0:%.*]] = load ptr, ptr [[P_ADDR]], align 8 +// CHECK-NEXT: [[TMP1:%.*]] = load i16, ptr [[X_ADDR]], align 2 +// CHECK-NEXT: [[TMP2:%.*]] = call i16 asm "", "=r,0,~{dirflag},~{fpsr},~{flags}"(i16 [[TMP1]]) #[[ATTR1]], !srcloc [[META4:![0-9]+]] +// CHECK-NEXT: store i16 [[TMP2]], ptr [[TMP0]], align 1 +// CHECK-NEXT: ret void +// +void test_tied_same_size(struct C *p, short x) { + asm("" : "=r"(*p) : "0"(x)); +} >From d0740abc17e104813fd82217d1f8c0316eaf08e9 Mon Sep 17 00:00:00 2001 From: Bill Wendling <[email protected]> Date: Thu, 29 Oct 2026 21:10:57 -0700 Subject: [PATCH 4/5] Reword the TBAA comment to describe the rule Assisted-by: Claude Opus 5.5 --- clang/lib/CodeGen/CGStmt.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp index dc34bfc0f5e2c6..347bf6b23cbc81 100644 --- a/clang/lib/CodeGen/CGStmt.cpp +++ b/clang/lib/CodeGen/CGStmt.cpp @@ -3389,9 +3389,9 @@ void AsmConstraintsInfo::EmitAsmStores( return; } - // The integer isn't the destination's type, so a TBAA tag for it would - // claim that the store doesn't alias accesses to the destination's - // members. + // The store writes all of the destination's members at once, so it must + // be allowed to alias an access to any of them. A TBAA access type must + // be a scalar, so use may-alias (char), which aliases any access. Dest = CGF.MakeAddrLValue(A, Ty, Dest.getBaseInfo(), TBAAAccessInfo::getMayAliasInfo()); } >From 6e5450392ae442187d957c38f286471e345a2656 Mon Sep 17 00:00:00 2001 From: Bill Wendling <[email protected]> Date: Thu, 29 Oct 2026 21:23:04 -0700 Subject: [PATCH 5/5] Convert any output type to an integer before truncating Sema only lets a struct or _Complex output be tied to a larger integer or pointer input, but X86's "k" constraint makes the type a vector of i1. A bit-or-pointer cast to an integer of the same size handles all of them. Assisted-by: Claude Opus 5.5 --- clang/lib/CodeGen/CGStmt.cpp | 13 +++++++------ clang/test/CodeGen/X86/inline-asm-output-store.c | 5 +++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp index b9bc6252b38bc3..d979cd7c67b9ef 100644 --- a/clang/lib/CodeGen/CGStmt.cpp +++ b/clang/lib/CodeGen/CGStmt.cpp @@ -3390,13 +3390,14 @@ void AsmConstraintsInfo::EmitAsmStores( return; } - // An output tied to a larger input has the input's type, so store only - // the destination's bits. + // An output tied to a larger input has the input's type, which may not be + // an integer (e.g., a pointer). Convert it to an integer of the same + // size, then store only the destination's bits. llvm::Type *IntTy = llvm::IntegerType::get(CTX, Size); - if (Tmp->getType()->isPointerTy()) - Tmp = Builder.CreatePtrToInt(Tmp, IntTy); - else - Tmp = Builder.CreateTrunc(Tmp, IntTy); + uint64_t TmpSize = CGM.getDataLayout().getTypeSizeInBits(Tmp->getType()); + Tmp = Builder.CreateBitOrPointerCast( + Tmp, llvm::IntegerType::get(CTX, (unsigned)TmpSize)); + Tmp = Builder.CreateTrunc(Tmp, IntTy); // The store writes all of the destination's members at once, so it must // be allowed to alias an access to any of them. A TBAA access type must diff --git a/clang/test/CodeGen/X86/inline-asm-output-store.c b/clang/test/CodeGen/X86/inline-asm-output-store.c index 819067b2e0f5e3..6389a42498a411 100644 --- a/clang/test/CodeGen/X86/inline-asm-output-store.c +++ b/clang/test/CodeGen/X86/inline-asm-output-store.c @@ -57,8 +57,9 @@ void test_tied_int(struct C *p, int x) { // CHECK-NEXT: [[TMP0:%.*]] = load ptr, ptr [[P_ADDR]], align 8 // CHECK-NEXT: [[TMP1:%.*]] = load ptr, ptr [[X_ADDR]], align 8 // CHECK-NEXT: [[TMP2:%.*]] = call ptr asm "", "=r,0,~{dirflag},~{fpsr},~{flags}"(ptr [[TMP1]]) #[[ATTR1]], !srcloc [[META3:![0-9]+]] -// CHECK-NEXT: [[TMP3:%.*]] = ptrtoint ptr [[TMP2]] to i16 -// CHECK-NEXT: store i16 [[TMP3]], ptr [[TMP0]], align 1 +// CHECK-NEXT: [[TMP3:%.*]] = ptrtoint ptr [[TMP2]] to i64 +// CHECK-NEXT: [[TMP4:%.*]] = trunc i64 [[TMP3]] to i16 +// CHECK-NEXT: store i16 [[TMP4]], ptr [[TMP0]], align 1 // CHECK-NEXT: ret void // void test_tied_pointer(struct C *p, void *x) { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
