[TBAA] Emit "omnipotent char" for intrinsics with type cast #107793

huhu233 · 2024-09-09T01:15:00Z

For the case,

  long long res2[SIZE];
  svst1(pa, (long *)&res2[0], v2);
  /* use res2[i] */

svst1 is emitted with TBAA metadata for "long", but other users of "res2" use "long long", which is a strict aliasing violation and may cause incorrect optimization like #97783. The root cause is we explictly cast the type of res2 when calling svst1, and the compiler emits an improper type. This patch fixes the case by emitting "omnipotent char" for intrinsics with type cast.

For the case, long long res2[SIZE]; svst1(pa, (long *)&res2[0], v2); /* use res2[i] */ svst1 is emitted with TBAA metadata for "long", but other users of "res2" use "long long", which is a strict aliasing violation and may cause incorrect optimization like llvm#97783. The root cause is we explictly cast the type of res2 when calling svst1, and the compiler emits an improper type. This patch fixes the case by emitting "omnipotent char" for intrinsics with type cast.

llvmbot · 2024-09-09T01:15:33Z

@llvm/pr-subscribers-clang

Author: None (huhu233)

Changes

For the case,

  long long res2[SIZE];
  svst1(pa, (long *)&amp;res2[0], v2);
  /* use res2[i] */

svst1 is emitted with TBAA metadata for "long", but other users of "res2" use "long long", which is a strict aliasing violation and may cause incorrect optimization like #97783. The root cause is we explictly cast the type of res2 when calling svst1, and the compiler emits an improper type. This patch fixes the case by emitting "omnipotent char" for intrinsics with type cast.

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

6 Files Affected:

(modified) clang/lib/CodeGen/CGBuiltin.cpp (+10)
(modified) clang/lib/CodeGen/CodeGenModule.cpp (+4)
(modified) clang/lib/CodeGen/CodeGenModule.h (+2)
(modified) clang/lib/CodeGen/CodeGenTBAA.cpp (+5)
(modified) clang/lib/CodeGen/CodeGenTBAA.h (+2)
(added) clang/test/CodeGen/tbaa-sve-store-acle.c (+17)

diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index da7a1a55da5313..27fd5ce976bec5 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -10155,6 +10155,11 @@ Value *CodeGenFunction::EmitSVEMaskedLoad(const CallExpr *E,
   auto *Load =
       cast<llvm::Instruction>(Builder.CreateCall(F, {Predicate, BasePtr}));
   auto TBAAInfo = CGM.getTBAAAccessInfo(LangPTy->getPointeeType());
+  if (auto *CastE = dyn_cast<CastExpr>(E->getArg(1))) {
+    CastKind CK = CastE->getCastKind();
+    if (CK == CK_BitCast)
+      TBAAInfo = CGM.genConservativeTBAA(LangPTy->getPointeeType());
+  }
   CGM.DecorateInstructionWithTBAA(Load, TBAAInfo);
 
   if (IsQuadLoad)
@@ -10207,6 +10212,11 @@ Value *CodeGenFunction::EmitSVEMaskedStore(const CallExpr *E,
   auto *Store =
       cast<llvm::Instruction>(Builder.CreateCall(F, {Val, Predicate, BasePtr}));
   auto TBAAInfo = CGM.getTBAAAccessInfo(LangPTy->getPointeeType());
+  if (auto *CastE = dyn_cast<CastExpr>(E->getArg(1))) {
+    CastKind CK = CastE->getCastKind();
+    if (CK == CK_BitCast)
+      TBAAInfo = CGM.genConservativeTBAA(LangPTy->getPointeeType());
+  }
   CGM.DecorateInstructionWithTBAA(Store, TBAAInfo);
   return Store;
 }
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index df4c13c9ad97aa..7c25591d08a92a 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1499,6 +1499,10 @@ TBAAAccessInfo CodeGenModule::getTBAAAccessInfo(QualType AccessType) {
   return TBAA->getAccessInfo(AccessType);
 }
 
+TBAAAccessInfo CodeGenModule::genConservativeTBAA(QualType AccessType) {
+  return TBAA->genConservativeTBAA(AccessType);
+}
+
 TBAAAccessInfo
 CodeGenModule::getTBAAVTablePtrAccessInfo(llvm::Type *VTablePtrType) {
   if (!TBAA)
diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h
index c58bb88035ca8a..2c30429850907e 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -811,6 +811,8 @@ class CodeGenModule : public CodeGenTypeCache {
   /// an object of the given type.
   TBAAAccessInfo getTBAAAccessInfo(QualType AccessType);
 
+  TBAAAccessInfo genConservativeTBAA(QualType AccessType);
+
   /// getTBAAVTablePtrAccessInfo - Get the TBAA information that describes an
   /// access to a virtual table pointer.
   TBAAAccessInfo getTBAAVTablePtrAccessInfo(llvm::Type *VTablePtrType);
diff --git a/clang/lib/CodeGen/CodeGenTBAA.cpp b/clang/lib/CodeGen/CodeGenTBAA.cpp
index 5b3393ec150e44..be285ba0dbff2a 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.cpp
+++ b/clang/lib/CodeGen/CodeGenTBAA.cpp
@@ -306,6 +306,11 @@ llvm::MDNode *CodeGenTBAA::getTypeInfo(QualType QTy) {
   return MetadataCache[Ty] = TypeNode;
 }
 
+TBAAAccessInfo CodeGenTBAA::genConservativeTBAA(QualType AccessType) {
+  uint64_t Size = Context.getTypeSizeInChars(AccessType).getQuantity();
+  return TBAAAccessInfo(getChar(), Size);
+}
+
 TBAAAccessInfo CodeGenTBAA::getAccessInfo(QualType AccessType) {
   // Pointee values may have incomplete types, but they shall never be
   // dereferenced.
diff --git a/clang/lib/CodeGen/CodeGenTBAA.h b/clang/lib/CodeGen/CodeGenTBAA.h
index ba74a39a4d25ee..de89783bc13e17 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.h
+++ b/clang/lib/CodeGen/CodeGenTBAA.h
@@ -213,6 +213,8 @@ class CodeGenTBAA {
   /// purpose of memory transfer calls.
   TBAAAccessInfo mergeTBAAInfoForMemoryTransfer(TBAAAccessInfo DestInfo,
                                                 TBAAAccessInfo SrcInfo);
+
+  TBAAAccessInfo genConservativeTBAA(QualType AccessType);
 };
 
 }  // end namespace CodeGen
diff --git a/clang/test/CodeGen/tbaa-sve-store-acle.c b/clang/test/CodeGen/tbaa-sve-store-acle.c
new file mode 100644
index 00000000000000..e9078ecb4370ad
--- /dev/null
+++ b/clang/test/CodeGen/tbaa-sve-store-acle.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -O1  %s \
+// RUN:     -emit-llvm -o - | FileCheck %s -check-prefix=TBAA
+#include <arm_sve.h>
+
+// TBAA:    store <vscale x 2 x i64>
+// TBAA:    !tbaa ![[TBAA6:[0-9]+]]
+long long sveStoreWithTypeCast(int *datas) {
+  long long res2[16];
+  svbool_t pa = svptrue_b32();
+  svint32_t v1 = svld1(pa, &datas[0]);
+  svint64_t v2 = svunpklo(v1);
+  svst1(pa, (long *)&res2[0], v2);
+  return res2[0] + res2[1];
+}
+
+// TBAA: ![[CHAR:[0-9]+]] = !{!"omnipotent char",
+// TBAA: ![[TBAA6:[0-9]+]] = !{![[CHAR]], ![[CHAR]], i64 0}

llvmbot · 2024-09-09T01:15:34Z

@llvm/pr-subscribers-clang-codegen

Author: None (huhu233)

Changes

For the case,

  long long res2[SIZE];
  svst1(pa, (long *)&amp;res2[0], v2);
  /* use res2[i] */

svst1 is emitted with TBAA metadata for "long", but other users of "res2" use "long long", which is a strict aliasing violation and may cause incorrect optimization like #97783. The root cause is we explictly cast the type of res2 when calling svst1, and the compiler emits an improper type. This patch fixes the case by emitting "omnipotent char" for intrinsics with type cast.

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

6 Files Affected:

(modified) clang/lib/CodeGen/CGBuiltin.cpp (+10)
(modified) clang/lib/CodeGen/CodeGenModule.cpp (+4)
(modified) clang/lib/CodeGen/CodeGenModule.h (+2)
(modified) clang/lib/CodeGen/CodeGenTBAA.cpp (+5)
(modified) clang/lib/CodeGen/CodeGenTBAA.h (+2)
(added) clang/test/CodeGen/tbaa-sve-store-acle.c (+17)

diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index da7a1a55da5313..27fd5ce976bec5 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -10155,6 +10155,11 @@ Value *CodeGenFunction::EmitSVEMaskedLoad(const CallExpr *E,
   auto *Load =
       cast<llvm::Instruction>(Builder.CreateCall(F, {Predicate, BasePtr}));
   auto TBAAInfo = CGM.getTBAAAccessInfo(LangPTy->getPointeeType());
+  if (auto *CastE = dyn_cast<CastExpr>(E->getArg(1))) {
+    CastKind CK = CastE->getCastKind();
+    if (CK == CK_BitCast)
+      TBAAInfo = CGM.genConservativeTBAA(LangPTy->getPointeeType());
+  }
   CGM.DecorateInstructionWithTBAA(Load, TBAAInfo);
 
   if (IsQuadLoad)
@@ -10207,6 +10212,11 @@ Value *CodeGenFunction::EmitSVEMaskedStore(const CallExpr *E,
   auto *Store =
       cast<llvm::Instruction>(Builder.CreateCall(F, {Val, Predicate, BasePtr}));
   auto TBAAInfo = CGM.getTBAAAccessInfo(LangPTy->getPointeeType());
+  if (auto *CastE = dyn_cast<CastExpr>(E->getArg(1))) {
+    CastKind CK = CastE->getCastKind();
+    if (CK == CK_BitCast)
+      TBAAInfo = CGM.genConservativeTBAA(LangPTy->getPointeeType());
+  }
   CGM.DecorateInstructionWithTBAA(Store, TBAAInfo);
   return Store;
 }
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index df4c13c9ad97aa..7c25591d08a92a 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1499,6 +1499,10 @@ TBAAAccessInfo CodeGenModule::getTBAAAccessInfo(QualType AccessType) {
   return TBAA->getAccessInfo(AccessType);
 }
 
+TBAAAccessInfo CodeGenModule::genConservativeTBAA(QualType AccessType) {
+  return TBAA->genConservativeTBAA(AccessType);
+}
+
 TBAAAccessInfo
 CodeGenModule::getTBAAVTablePtrAccessInfo(llvm::Type *VTablePtrType) {
   if (!TBAA)
diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h
index c58bb88035ca8a..2c30429850907e 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -811,6 +811,8 @@ class CodeGenModule : public CodeGenTypeCache {
   /// an object of the given type.
   TBAAAccessInfo getTBAAAccessInfo(QualType AccessType);
 
+  TBAAAccessInfo genConservativeTBAA(QualType AccessType);
+
   /// getTBAAVTablePtrAccessInfo - Get the TBAA information that describes an
   /// access to a virtual table pointer.
   TBAAAccessInfo getTBAAVTablePtrAccessInfo(llvm::Type *VTablePtrType);
diff --git a/clang/lib/CodeGen/CodeGenTBAA.cpp b/clang/lib/CodeGen/CodeGenTBAA.cpp
index 5b3393ec150e44..be285ba0dbff2a 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.cpp
+++ b/clang/lib/CodeGen/CodeGenTBAA.cpp
@@ -306,6 +306,11 @@ llvm::MDNode *CodeGenTBAA::getTypeInfo(QualType QTy) {
   return MetadataCache[Ty] = TypeNode;
 }
 
+TBAAAccessInfo CodeGenTBAA::genConservativeTBAA(QualType AccessType) {
+  uint64_t Size = Context.getTypeSizeInChars(AccessType).getQuantity();
+  return TBAAAccessInfo(getChar(), Size);
+}
+
 TBAAAccessInfo CodeGenTBAA::getAccessInfo(QualType AccessType) {
   // Pointee values may have incomplete types, but they shall never be
   // dereferenced.
diff --git a/clang/lib/CodeGen/CodeGenTBAA.h b/clang/lib/CodeGen/CodeGenTBAA.h
index ba74a39a4d25ee..de89783bc13e17 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.h
+++ b/clang/lib/CodeGen/CodeGenTBAA.h
@@ -213,6 +213,8 @@ class CodeGenTBAA {
   /// purpose of memory transfer calls.
   TBAAAccessInfo mergeTBAAInfoForMemoryTransfer(TBAAAccessInfo DestInfo,
                                                 TBAAAccessInfo SrcInfo);
+
+  TBAAAccessInfo genConservativeTBAA(QualType AccessType);
 };
 
 }  // end namespace CodeGen
diff --git a/clang/test/CodeGen/tbaa-sve-store-acle.c b/clang/test/CodeGen/tbaa-sve-store-acle.c
new file mode 100644
index 00000000000000..e9078ecb4370ad
--- /dev/null
+++ b/clang/test/CodeGen/tbaa-sve-store-acle.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -O1  %s \
+// RUN:     -emit-llvm -o - | FileCheck %s -check-prefix=TBAA
+#include <arm_sve.h>
+
+// TBAA:    store <vscale x 2 x i64>
+// TBAA:    !tbaa ![[TBAA6:[0-9]+]]
+long long sveStoreWithTypeCast(int *datas) {
+  long long res2[16];
+  svbool_t pa = svptrue_b32();
+  svint32_t v1 = svld1(pa, &datas[0]);
+  svint64_t v2 = svunpklo(v1);
+  svst1(pa, (long *)&res2[0], v2);
+  return res2[0] + res2[1];
+}
+
+// TBAA: ![[CHAR:[0-9]+]] = !{!"omnipotent char",
+// TBAA: ![[TBAA6:[0-9]+]] = !{![[CHAR]], ![[CHAR]], i64 0}

arsenm · 2024-09-09T04:10:51Z

I don't understand this. The code is a strict aliasing violation, so why should clang work around it?

huhu233 · 2024-09-09T06:19:09Z

I don't understand this. The code is a strict aliasing violation, so why should clang work around it?

Hi, @arsenm, the violation is due to the compiler emitting different TBAA for svst1 (long) and other users of res2 (long long). TBAA is invisible to ACLE users, and the compiler should tell them that this kind of code may cause errors, or work around it (e.g., emit omnipotent char to svst1 if there is bitcast).

efriedma-quic · 2024-09-09T18:07:33Z

See https://reviews.llvm.org/D119319; CC @sdesmalen-arm @paulwalker-arm

paulwalker-arm · 2024-09-10T20:07:03Z

I'll need to look to see if the TBAA metadata we're adding matches what we initially envisaged but my gut feeling matches @arsenm.

I'm assuming the original code emitted an error, something along the lines of "no version of svst1 available for long long*", which you've "fixed" by introducing a cast that breaks strict aliasing rules.

I recall warnings relating to strict aliasing violations being hit and miss? but perhaps we can do better.

huhu233 · 2024-09-11T01:22:40Z

I'll need to look to see if the TBAA metadata we're adding matches what we initially envisaged but my gut feeling matches @arsenm.

I'm assuming the original code emitted an error, something along the lines of "no version of svst1 available for long long*", which you've "fixed" by introducing a cast that breaks strict aliasing rules.

I recall warnings relating to strict aliasing violations being hit and miss? but perhaps we can do better.

Hi, @paulwalker-arm, ACLE allows users to do instruction-level development, but mixing intrinsic and regular C code may break some of the rules set by the compiler. For example, in this case, the explict cast causes res2 to have different tbaa in different contexts. The compiler needs to detect these behaviors and provide some hints, or work around them to avoid a series of wrong optimizations that follow.

arsenm · 2024-09-11T10:06:12Z

Hi, @paulwalker-arm, ACLE allows users to do instruction-level development, but mixing intrinsic and regular C code may break some of the rules set by the compiler.

The rules are still there. You can always use a union or copy to avoid violating the rules. I don't think it makes sense to special case any intrinsics

huhu233 added 2 commits September 8, 2024 12:59

[TBAA] Precommit a test for tbaa enhancement

433a809

llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels Sep 9, 2024

huhu233 linked an issue Sep 9, 2024 that may be closed by this pull request

[TBAA] "long long" type causes incorrect optimization in GVN #97783

Open

huhu233 requested a review from arsenm September 9, 2024 01:19

huhu233 requested review from efriedma-quic, paulwalker-arm and sdesmalen-arm September 10, 2024 01:24

huhu233 added the TBAA Type-Based Alias Analysis / Strict Aliasing label Sep 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[TBAA] Emit "omnipotent char" for intrinsics with type cast #107793

[TBAA] Emit "omnipotent char" for intrinsics with type cast #107793

Uh oh!

huhu233 commented Sep 9, 2024

Uh oh!

llvmbot commented Sep 9, 2024

Uh oh!

llvmbot commented Sep 9, 2024

Uh oh!

arsenm commented Sep 9, 2024

Uh oh!

huhu233 commented Sep 9, 2024

Uh oh!

efriedma-quic commented Sep 9, 2024

Uh oh!

paulwalker-arm commented Sep 10, 2024

Uh oh!

huhu233 commented Sep 11, 2024

Uh oh!

arsenm commented Sep 11, 2024

Uh oh!

Uh oh!

[TBAA] Emit "omnipotent char" for intrinsics with type cast #107793

Are you sure you want to change the base?

[TBAA] Emit "omnipotent char" for intrinsics with type cast #107793

Uh oh!

Conversation

huhu233 commented Sep 9, 2024

Uh oh!

llvmbot commented Sep 9, 2024

Uh oh!

llvmbot commented Sep 9, 2024

Uh oh!

arsenm commented Sep 9, 2024

Uh oh!

huhu233 commented Sep 9, 2024

Uh oh!

efriedma-quic commented Sep 9, 2024

Uh oh!

paulwalker-arm commented Sep 10, 2024

Uh oh!

huhu233 commented Sep 11, 2024

Uh oh!

arsenm commented Sep 11, 2024

Uh oh!

Uh oh!