https://github.com/fhahn updated
https://github.com/llvm/llvm-project/pull/122116
>From befe3a63d95712bf450ad1af360a8fb50c655a12 Mon Sep 17 00:00:00 2001
From: Florian Hahn <f...@fhahn.com>
Date: Wed, 8 Jan 2025 13:07:01 +0000
Subject: [PATCH 1/2] [TBAA] Don't emit pointer-tbaa for void pointers.
While there are no special rules in the standards regarding
void pointers and strict aliasing, emitting distinct tags for void
pointers break some common idioms and there is no good alternative to
re-write the code without strict-aliasing violations. An example is
to count the entries in an array of pointers:
int count_elements(void * values) {
void **seq = values;
int count;
for (count = 0; seq && seq[count]; count++);
return count;
}
https://clang.godbolt.org/z/8dTv51v8W
An example in the wild is from
https://github.com/llvm/llvm-project/issues/119099
This patch avoids emitting distinct tags for void pointers, to avoid
those idioms causing mis-compiles for now.
---
clang/lib/CodeGen/CodeGenTBAA.cpp | 8 ++++++++
clang/test/CodeGen/tbaa-pointers.c | 13 +++----------
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenTBAA.cpp
b/clang/lib/CodeGen/CodeGenTBAA.cpp
index 75e66bae79afdc..3f1a24791ddd81 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.cpp
+++ b/clang/lib/CodeGen/CodeGenTBAA.cpp
@@ -226,6 +226,14 @@ llvm::MDNode *CodeGenTBAA::getTypeInfoHelper(const Type
*Ty) {
PtrDepth++;
Ty = Ty->getPointeeType()->getBaseElementTypeUnsafe();
} while (Ty->isPointerType());
+
+ // While there are no special rules in the standards regarding void
pointers
+ // and strict aliasing, emitting distinct tags for void pointers break some
+ // common idioms and there is no good alternative to re-write the code
+ // without strict-aliasing violations.
+ if (Ty->isVoidType())
+ return AnyPtr;
+
assert(!isa<VariableArrayType>(Ty));
// When the underlying type is a builtin type, we compute the pointee type
// string recursively, which is implicitly more forgiving than the
standards
diff --git a/clang/test/CodeGen/tbaa-pointers.c
b/clang/test/CodeGen/tbaa-pointers.c
index 4aae2552f107a3..48adac503357f0 100644
--- a/clang/test/CodeGen/tbaa-pointers.c
+++ b/clang/test/CodeGen/tbaa-pointers.c
@@ -208,12 +208,9 @@ int void_ptrs(void **ptr) {
// COMMON-LABEL: define i32 @void_ptrs(
// COMMON-SAME: ptr noundef [[PTRA:%.+]])
// COMMON: [[PTR_ADDR:%.+]] = alloca ptr, align 8
-// DISABLE-NEXT: store ptr [[PTRA]], ptr [[PTR_ADDR]], align 8, !tbaa
[[ANYPTR]]
-// DISABLE-NEXT: [[L0:%.+]] = load ptr, ptr [[PTR_ADDR]], align 8, !tbaa
[[ANYPTR]]
-// DISABLE-NEXT: [[L1:%.+]] = load ptr, ptr [[L0]], align 8, !tbaa [[ANYPTR]]
-// DEFAULT-NEXT: store ptr [[PTRA]], ptr [[PTR_ADDR]], align 8, !tbaa
[[P2VOID:!.+]]
-// DEFAULT-NEXT: [[L0:%.+]] = load ptr, ptr [[PTR_ADDR]], align 8, !tbaa
[[P2VOID]]
-// DEFAULT-NEXT: [[L1:%.+]] = load ptr, ptr [[L0]], align 8, !tbaa
[[P1VOID:!.+]]
+// COMMON-NEXT: store ptr [[PTRA]], ptr [[PTR_ADDR]], align 8, !tbaa
[[ANYPTR]]
+// COMMON-NEXT: [[L0:%.+]] = load ptr, ptr [[PTR_ADDR]], align 8, !tbaa
[[ANYPTR]]
+// COMMON-NEXT: [[L1:%.+]] = load ptr, ptr [[L0]], align 8, !tbaa [[ANYPTR]]
// COMMON-NEXT: [[BOOL:%.+]] = icmp ne ptr [[L1]], null
// COMMON-NEXT: [[BOOL_EXT:%.+]] = zext i1 [[BOOL]] to i64
// COMMON-NEXT: [[COND:%.+]] = select i1 [[BOOL]], i32 0, i32 1
@@ -254,7 +251,3 @@ int void_ptrs(void **ptr) {
// COMMON: [[INT_TAG]] = !{[[INT_TY:!.+]], [[INT_TY]], i64 0}
// COMMON: [[INT_TY]] = !{!"int", [[CHAR]], i64 0}
// DEFAULT: [[ANYPTR]] = !{[[ANY_POINTER]], [[ANY_POINTER]], i64 0}
-// DEFAULT: [[P2VOID]] = !{[[P2VOID_TY:!.+]], [[P2VOID_TY]], i64 0}
-// DEFAULT: [[P2VOID_TY]] = !{!"p2 void", [[ANY_POINTER]], i64 0}
-// DEFAULT: [[P1VOID]] = !{[[P1VOID_TY:!.+]], [[P1VOID_TY]], i64 0}
-// DEFAULT: [[P1VOID_TY]] = !{!"p1 void", [[ANY_POINTER]], i64 0}
>From 471ed86de267fb7b99b49d6187dc20d031eb4b7f Mon Sep 17 00:00:00 2001
From: Florian Hahn <f...@fhahn.com>
Date: Thu, 9 Jan 2025 11:49:55 +0000
Subject: [PATCH 2/2] !fixup update remaining tests accessing void *
---
.../CodeGenOpenCL/amdgpu-enqueue-kernel.cl | 25 +++++++++----------
clang/unittests/CodeGen/TBAAMetadataTest.cpp | 5 +---
2 files changed, 13 insertions(+), 17 deletions(-)
diff --git a/clang/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl
b/clang/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl
index 5599f4dd50f042..ace34dd0ca6dce 100644
--- a/clang/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl
+++ b/clang/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl
@@ -651,7 +651,7 @@ kernel void test_target_features_kernel(global int *i) {
//
// GFX900: Function Attrs: convergent nounwind
// GFX900-LABEL: define {{[^@]+}}@__test_block_invoke_3_kernel
-// GFX900-SAME: (<{ i32, i32, ptr, ptr addrspace(1), ptr addrspace(1), i64, i8
}> [[TMP0:%.*]], ptr addrspace(3) [[TMP1:%.*]]) #[[ATTR6]]
!kernel_arg_addr_space [[META28:![0-9]+]] !kernel_arg_access_qual
[[META29:![0-9]+]] !kernel_arg_type [[META30:![0-9]+]] !kernel_arg_base_type
[[META30]] !kernel_arg_type_qual [[META31:![0-9]+]] {
+// GFX900-SAME: (<{ i32, i32, ptr, ptr addrspace(1), ptr addrspace(1), i64, i8
}> [[TMP0:%.*]], ptr addrspace(3) [[TMP1:%.*]]) #[[ATTR6]]
!kernel_arg_addr_space [[META27:![0-9]+]] !kernel_arg_access_qual
[[META28:![0-9]+]] !kernel_arg_type [[META29:![0-9]+]] !kernel_arg_base_type
[[META29]] !kernel_arg_type_qual [[META30:![0-9]+]] {
// GFX900-NEXT: entry:
// GFX900-NEXT: [[TMP2:%.*]] = alloca <{ i32, i32, ptr, ptr addrspace(1),
ptr addrspace(1), i64, i8 }>, align 8, addrspace(5)
// GFX900-NEXT: store <{ i32, i32, ptr, ptr addrspace(1), ptr addrspace(1),
i64, i8 }> [[TMP0]], ptr addrspace(5) [[TMP2]], align 8
@@ -688,7 +688,7 @@ kernel void test_target_features_kernel(global int *i) {
//
// GFX900: Function Attrs: convergent norecurse nounwind
// GFX900-LABEL: define {{[^@]+}}@test_target_features_kernel
-// GFX900-SAME: (ptr addrspace(1) noundef align 4 [[I:%.*]]) #[[ATTR2]]
!kernel_arg_addr_space [[META32:![0-9]+]] !kernel_arg_access_qual [[META23]]
!kernel_arg_type [[META33:![0-9]+]] !kernel_arg_base_type [[META33]]
!kernel_arg_type_qual [[META25]] {
+// GFX900-SAME: (ptr addrspace(1) noundef align 4 [[I:%.*]]) #[[ATTR2]]
!kernel_arg_addr_space [[META31:![0-9]+]] !kernel_arg_access_qual [[META23]]
!kernel_arg_type [[META32:![0-9]+]] !kernel_arg_base_type [[META32]]
!kernel_arg_type_qual [[META25]] {
// GFX900-NEXT: entry:
// GFX900-NEXT: [[I_ADDR:%.*]] = alloca ptr addrspace(1), align 8,
addrspace(5)
// GFX900-NEXT: [[DEFAULT_QUEUE:%.*]] = alloca ptr addrspace(1), align 8,
addrspace(5)
@@ -700,7 +700,7 @@ kernel void test_target_features_kernel(global int *i) {
// GFX900-NEXT: [[FLAGS_ASCAST:%.*]] = addrspacecast ptr addrspace(5)
[[FLAGS]] to ptr
// GFX900-NEXT: [[NDRANGE_ASCAST:%.*]] = addrspacecast ptr addrspace(5)
[[NDRANGE]] to ptr
// GFX900-NEXT: [[TMP_ASCAST:%.*]] = addrspacecast ptr addrspace(5) [[TMP]]
to ptr
-// GFX900-NEXT: store ptr addrspace(1) [[I]], ptr [[I_ADDR_ASCAST]], align
8, !tbaa [[TBAA34:![0-9]+]]
+// GFX900-NEXT: store ptr addrspace(1) [[I]], ptr [[I_ADDR_ASCAST]], align
8, !tbaa [[TBAA33:![0-9]+]]
// GFX900-NEXT: call void @llvm.lifetime.start.p5(i64 8, ptr addrspace(5)
[[DEFAULT_QUEUE]]) #[[ATTR8]]
// GFX900-NEXT: call void @llvm.lifetime.start.p5(i64 4, ptr addrspace(5)
[[FLAGS]]) #[[ATTR8]]
// GFX900-NEXT: store i32 0, ptr [[FLAGS_ASCAST]], align 4, !tbaa [[TBAA17]]
@@ -803,16 +803,15 @@ kernel void test_target_features_kernel(global int *i) {
// GFX900: [[META23]] = !{!"none"}
// GFX900: [[META24]] = !{!"__block_literal"}
// GFX900: [[META25]] = !{!""}
-// GFX900: [[TBAA26]] = !{[[META27:![0-9]+]], [[META27]], i64 0}
-// GFX900: [[META27]] = !{!"p1 void", [[META9]], i64 0}
-// GFX900: [[META28]] = !{i32 0, i32 3}
-// GFX900: [[META29]] = !{!"none", !"none"}
-// GFX900: [[META30]] = !{!"__block_literal", !"void*"}
-// GFX900: [[META31]] = !{!"", !""}
-// GFX900: [[META32]] = !{i32 1}
-// GFX900: [[META33]] = !{!"int*"}
-// GFX900: [[TBAA34]] = !{[[META35:![0-9]+]], [[META35]], i64 0}
-// GFX900: [[META35]] = !{!"p1 int", [[META9]], i64 0}
+// GFX900: [[TBAA26]] = !{[[META9]], [[META9]], i64 0}
+// GFX900: [[META27]] = !{i32 0, i32 3}
+// GFX900: [[META28]] = !{!"none", !"none"}
+// GFX900: [[META29]] = !{!"__block_literal", !"void*"}
+// GFX900: [[META30]] = !{!"", !""}
+// GFX900: [[META31]] = !{i32 1}
+// GFX900: [[META32]] = !{!"int*"}
+// GFX900: [[TBAA33]] = !{[[META34:![0-9]+]], [[META34]], i64 0}
+// GFX900: [[META34]] = !{!"p1 int", [[META9]], i64 0}
//.
//// NOTE: These prefixes are unused and the list is autogenerated. Do not add
tests below this line:
// CHECK: {{.*}}
diff --git a/clang/unittests/CodeGen/TBAAMetadataTest.cpp
b/clang/unittests/CodeGen/TBAAMetadataTest.cpp
index cad8783ea73fba..35e68a577e08f6 100644
--- a/clang/unittests/CodeGen/TBAAMetadataTest.cpp
+++ b/clang/unittests/CodeGen/TBAAMetadataTest.cpp
@@ -120,10 +120,7 @@ TEST(TBAAMetadataTest, BasicTypes) {
MInstruction(Instruction::Store,
MValType(PointerType::getUnqual(Compiler.Context)),
MMTuple(
- MMTuple(
- MMString("p1 void"),
- AnyPtr,
- MConstInt(0)),
+ AnyPtr,
MSameAs(0),
MConstInt(0))));
ASSERT_TRUE(I);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
