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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. AlexVlx marked an inline comment as done. Closed by commit rG51a014cb2d9c: [Clang][CodeGen] `__builtin_alloca`s should care about address spaces (authored by AlexVlx).

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

rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. No problem, LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156539/new/ https://reviews.llvm.org/D156539 ___ cfe-commits mailing

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

AlexVlx marked an inline comment as done. AlexVlx added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3542 +LangAS AAS = getASTAllocaAddressSpace(); +LangAS EAS = E->getType().getAddressSpace(); +if (AAS != EAS) { rjmccall wrote: >

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

AlexVlx updated this revision to Diff 545882. AlexVlx added a comment. Use pointee's address space for the check. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156539/new/ https://reviews.llvm.org/D156539 Files: clang/lib/CodeGen/CGBuiltin.cpp

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

rjmccall added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3542 +LangAS AAS = getASTAllocaAddressSpace(); +LangAS EAS = E->getType().getAddressSpace(); +if (AAS != EAS) { `E->getType().getAddressSpace()` is actually the top-level

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

AlexVlx updated this revision to Diff 545860. AlexVlx added a comment. Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156539/new/ https://reviews.llvm.org/D156539 Files: clang/lib/CodeGen/CGBuiltin.cpp clang/test/CodeGen/dynamic-alloca-with-address-space.c

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

AlexVlx added a comment. In D156539#4547814 , @rjmccall wrote: > In D156539#4546834 , @AlexVlx wrote: > >> In D156539#4542836 , @rjmccall >> wrote: >> >>> We should

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

rjmccall added a comment. In D156539#4546834 , @AlexVlx wrote: > In D156539#4542836 , @rjmccall > wrote: > >> We should probably write this code to work properly in case we add a target >> that makes

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

arsenm added inline comments. Comment at: clang/test/CodeGen/dynamic-alloca-with-address-space.c:1 +// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm %s -o - | FileCheck %s + AlexVlx wrote: > arsenm wrote: > > Can you add an opencl 1.2 and 2.0 run line too

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

AlexVlx added inline comments. Comment at: clang/test/CodeGen/dynamic-alloca-with-address-space.c:1 +// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm %s -o - | FileCheck %s + arsenm wrote: > Can you add an opencl 1.2 and 2.0 run line too This is not valid

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

arsenm added inline comments. Comment at: clang/test/CodeGen/dynamic-alloca-with-address-space.c:1 +// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm %s -o - | FileCheck %s + Can you add an opencl 1.2 and 2.0 run line too Comment at:

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

AlexVlx updated this revision to Diff 545679. AlexVlx added a reviewer: arsenm. AlexVlx added a comment. Herald added a subscriber: wdng. Incorporated review feedback. Updated test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156539/new/ https://reviews.llvm.org/D156539 Files:

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

AlexVlx added a comment. In D156539#4542836 , @rjmccall wrote: > We should probably write this code to work properly in case we add a target > that makes `__builtin_alloca` return a pointer in the private address space. > Could you recover the target

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

arsenm added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3540 + return RValue::get(Builder.CreateAddrSpaceCast(AI, CGM.Int8PtrTy)); +else + return RValue::get(AI); No return after else Comment at:

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

arsenm added a comment. In D156539#4542836 , @rjmccall wrote: > We should probably write this code to work properly in case we add a target > that makes `__builtin_alloca` return a pointer in the private address space. > Could you recover the target

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

rjmccall added a comment. We should probably write this code to work properly in case we add a target that makes `__builtin_alloca` return a pointer in the private address space. Could you recover the target AS from the type of the expression instead of assuming `LangAS::Default`?

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

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