[PATCH] D156539: [Clang][CodeGen] `__builtin_alloca`s should care about address spaces too

[Alex Voicu via Phabricator via cfe-commits]

AlexVlx created this revision.
AlexVlx added reviewers: rjmccall, yaxunl.
AlexVlx added a project: clang.
Herald added a subscriber: arichardson.
Herald added a project: All.
AlexVlx requested review of this revision.
Herald added a subscriber: cfe-commits.

`alloca` instructions always return pointers to the stack / alloca address space. This composes poorly with most HLLs which are address space agnostic and thus have all pointers point to generic/flat. Static `alloca`s were already handled on the AST level, in the implementation of the `CreateTempAlloca` family of functions, however dynamic allocas were not, which would make the following valid C++ code:

  const char* p = static_cast<const char*>(__builtin_alloca(42));

lead to subtly incorrect IR / an assert flaring. This patch addresses that by inserting an address space cast iff the alloca address space is different from generic.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156539

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/dynamic-alloca-with-address-space.c


Index: clang/test/CodeGen/dynamic-alloca-with-address-space.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/dynamic-alloca-with-address-space.c
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm %s -o - | FileCheck %s
+
+void allocas(unsigned long n) {
+    void *a = __builtin_alloca(n);
+    void *uninitialized_a = __builtin_alloca_uninitialized(n);
+    void *aligned_a = __builtin_alloca_with_align(n, 8);
+    void *aligned_uninitialized_a = __builtin_alloca_with_align_uninitialized(n, 8);
+}
+
+// CHECK: @allocas(
+// CHECK: store i64 %n, ptr %n.addr.ascast, align 8
+// CHECK: %0 = load i64, ptr %n.addr.ascast, align 8
+// CHECK: %1 = alloca i8, i64 %0, align 8, addrspace(5)
+// CHECK: %2 = addrspacecast ptr addrspace(5) %1 to ptr
+// CHECK: store ptr %2, ptr %a.ascast, align 8
+// CHECK: %3 = load i64, ptr %n.addr.ascast, align 8
+// CHECK: %4 = alloca i8, i64 %3, align 8, addrspace(5)
+// CHECK: %5 = addrspacecast ptr addrspace(5) %4 to ptr
+// CHECK: store ptr %5, ptr %uninitialized_a.ascast, align 8
+// CHECK: %6 = load i64, ptr %n.addr.ascast, align 8
+// CHECK: %7 = alloca i8, i64 %6, align 1, addrspace(5)
+// CHECK: %8 = addrspacecast ptr addrspace(5) %7 to ptr
+// CHECK: store ptr %8, ptr %aligned_a.ascast, align 8
+// CHECK: %9 = load i64, ptr %n.addr.ascast, align 8
+// CHECK: %10 = alloca i8, i64 %9, align 1, addrspace(5)
+// CHECK: %11 = addrspacecast ptr addrspace(5) %10 to ptr
+// CHECK: store ptr %11, ptr %aligned_uninitialized_a.ascast, align 8
+// CHECK: ret void
Index: clang/lib/CodeGen/CGBuiltin.cpp
===================================================================
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -3514,6 +3514,12 @@
     return RValue::get(Result);
   }
 
+  // An alloca will always return a pointer to the alloca (stack) address
+  // space. This address space need not be the same as the AST / Language
+  // default (e.g. in C / C++ auto vars are in the generic address space). At
+  // the AST level this is handled within CreateTempAlloca et al., but for the
+  // builtin / dynamic alloca we have to handle it here. We use an explicit cast
+  // instead of passing an AS to CreateAlloca so as to not inhibit optimisation.
   case Builtin::BIalloca:
   case Builtin::BI_alloca:
   case Builtin::BI__builtin_alloca_uninitialized:
@@ -3529,7 +3535,10 @@
     AI->setAlignment(SuitableAlignmentInBytes);
     if (BuiltinID != Builtin::BI__builtin_alloca_uninitialized)
       initializeAlloca(*this, AI, Size, SuitableAlignmentInBytes);
-    return RValue::get(AI);
+    if (getASTAllocaAddressSpace() != LangAS::Default)
+      return RValue::get(Builder.CreateAddrSpaceCast(AI, CGM.Int8PtrTy));
+    else
+      return RValue::get(AI);
   }
 
   case Builtin::BI__builtin_alloca_with_align_uninitialized:
@@ -3544,7 +3553,10 @@
     AI->setAlignment(AlignmentInBytes);
     if (BuiltinID != Builtin::BI__builtin_alloca_with_align_uninitialized)
       initializeAlloca(*this, AI, Size, AlignmentInBytes);
-    return RValue::get(AI);
+    if (getASTAllocaAddressSpace() != LangAS::Default)
+      return RValue::get(Builder.CreateAddrSpaceCast(AI, CGM.Int8PtrTy));
+    else
+      return RValue::get(AI);
   }
 
   case Builtin::BIbzero:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D156539.545145.patch
Type: text/x-patch
Size: 3320 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230728/562f28eb/attachment-0001.bin>