[PATCH] D47099: Disable casting of alloca for ActiveFlag
yaxunl updated this revision to Diff 147914.
yaxunl edited the summary of this revision.
yaxunl added a comment.
Revised by John's comments.
https://reviews.llvm.org/D47099
Files:
lib/CodeGen/CGCall.cpp
lib/CodeGen/CGCleanup.cpp
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CodeGenFunction.h
test/CodeGenCXX/conditional-temporaries.cpp
Index: test/CodeGenCXX/conditional-temporaries.cpp
===
--- test/CodeGenCXX/conditional-temporaries.cpp
+++ test/CodeGenCXX/conditional-temporaries.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-apple-darwin9 -O3 | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple=amdgcn-amd-amdhsa -O3 | FileCheck %s
namespace {
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2020,18 +2020,20 @@
/// to the stack.
///
/// Because the address of a temporary is often exposed to the program in
- /// various ways, this function will perform the cast by default. The cast
- /// may be avoided by passing false as \p CastToDefaultAddrSpace; this is
+ /// various ways, this function will perform the cast. The original alloca
+ /// instruction is returned through \p Alloca if it is not nullptr.
+ ///
+ /// The cast is not performaed in CreateTempAllocaWithoutCast. This is
/// more efficient if the caller knows that the address will not be exposed.
- /// The original alloca instruction is returned through \p Alloca if it is
- /// not nullptr.
llvm::AllocaInst *CreateTempAlloca(llvm::Type *Ty, const Twine = "tmp",
llvm::Value *ArraySize = nullptr);
Address CreateTempAlloca(llvm::Type *Ty, CharUnits align,
const Twine = "tmp",
llvm::Value *ArraySize = nullptr,
- Address *Alloca = nullptr,
- bool CastToDefaultAddrSpace = true);
+ Address *Alloca = nullptr);
+ Address CreateTempAllocaWithoutCast(llvm::Type *Ty, CharUnits align,
+ const Twine = "tmp",
+ llvm::Value *ArraySize = nullptr);
/// CreateDefaultAlignedTempAlloca - This creates an alloca with the
/// default ABI alignment of the given LLVM type.
@@ -2066,15 +2068,18 @@
Address CreateIRTemp(QualType T, const Twine = "tmp");
/// CreateMemTemp - Create a temporary memory object of the given type, with
- /// appropriate alignment. Cast it to the default address space if
- /// \p CastToDefaultAddrSpace is true. Returns the original alloca
- /// instruction by \p Alloca if it is not nullptr.
+ /// appropriate alignmen and cast it to the default address space. Returns
+ /// the original alloca instruction by \p Alloca if it is not nullptr.
Address CreateMemTemp(QualType T, const Twine = "tmp",
-Address *Alloca = nullptr,
-bool CastToDefaultAddrSpace = true);
+Address *Alloca = nullptr);
Address CreateMemTemp(QualType T, CharUnits Align, const Twine = "tmp",
-Address *Alloca = nullptr,
-bool CastToDefaultAddrSpace = true);
+Address *Alloca = nullptr);
+
+ /// CreateMemTemp - Create a temporary memory object of the given type, with
+ /// appropriate alignmen without casting it to the default address space.
+ Address CreateMemTempWithoutCast(QualType T, const Twine = "tmp");
+ Address CreateMemTempWithoutCast(QualType T, CharUnits Align,
+ const Twine = "tmp");
/// CreateAggTemp - Create a temporary memory object for the given
/// aggregate type.
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -61,21 +61,30 @@
/// CreateTempAlloca - This creates a alloca and inserts it into the entry
/// block.
+Address CodeGenFunction::CreateTempAllocaWithoutCast(llvm::Type *Ty,
+ CharUnits Align,
+ const Twine ,
+ llvm::Value *ArraySize) {
+ auto Alloca = CreateTempAlloca(Ty, Name, ArraySize);
+ Alloca->setAlignment(Align.getQuantity());
+ return Address(Alloca, Align);
+}
+
+/// CreateTempAlloca - This creates a alloca and inserts it into the entry
+/// block. The alloca is casted to default address space if necessary.
Address CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, CharUnits Align,
const Twine ,
llvm::Value *ArraySize,
- Address *AllocaAddr,
-
[PATCH] D47099: Disable casting of alloca for ActiveFlag
rjmccall added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:80 auto Alloca = CreateTempAlloca(Ty, Name, ArraySize); Alloca->setAlignment(Align.getQuantity()); if (AllocaAddr) Could you change this to call CreateTempAllocaWithoutCast? https://reviews.llvm.org/D47099 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47099: Disable casting of alloca for ActiveFlag
yaxunl updated this revision to Diff 147860.
yaxunl added a comment.
Add CreateMemTempWithoutCast and CreateTempAllocaWithoutCast by John's comments.
https://reviews.llvm.org/D47099
Files:
lib/CodeGen/CGCall.cpp
lib/CodeGen/CGCleanup.cpp
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CodeGenFunction.h
test/CodeGenCXX/conditional-temporaries.cpp
Index: test/CodeGenCXX/conditional-temporaries.cpp
===
--- test/CodeGenCXX/conditional-temporaries.cpp
+++ test/CodeGenCXX/conditional-temporaries.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-apple-darwin9 -O3 | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple=amdgcn-amd-amdhsa -O3 | FileCheck %s
namespace {
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2020,18 +2020,20 @@
/// to the stack.
///
/// Because the address of a temporary is often exposed to the program in
- /// various ways, this function will perform the cast by default. The cast
- /// may be avoided by passing false as \p CastToDefaultAddrSpace; this is
+ /// various ways, this function will perform the cast. The original alloca
+ /// instruction is returned through \p Alloca if it is not nullptr.
+ ///
+ /// The cast is not performaed in CreateTempAllocaWithoutCast. This is
/// more efficient if the caller knows that the address will not be exposed.
- /// The original alloca instruction is returned through \p Alloca if it is
- /// not nullptr.
llvm::AllocaInst *CreateTempAlloca(llvm::Type *Ty, const Twine = "tmp",
llvm::Value *ArraySize = nullptr);
Address CreateTempAlloca(llvm::Type *Ty, CharUnits align,
const Twine = "tmp",
llvm::Value *ArraySize = nullptr,
- Address *Alloca = nullptr,
- bool CastToDefaultAddrSpace = true);
+ Address *Alloca = nullptr);
+ Address CreateTempAllocaWithoutCast(llvm::Type *Ty, CharUnits align,
+ const Twine = "tmp",
+ llvm::Value *ArraySize = nullptr);
/// CreateDefaultAlignedTempAlloca - This creates an alloca with the
/// default ABI alignment of the given LLVM type.
@@ -2066,15 +2068,18 @@
Address CreateIRTemp(QualType T, const Twine = "tmp");
/// CreateMemTemp - Create a temporary memory object of the given type, with
- /// appropriate alignment. Cast it to the default address space if
- /// \p CastToDefaultAddrSpace is true. Returns the original alloca
- /// instruction by \p Alloca if it is not nullptr.
+ /// appropriate alignmen and cast it to the default address space. Returns
+ /// the original alloca instruction by \p Alloca if it is not nullptr.
Address CreateMemTemp(QualType T, const Twine = "tmp",
-Address *Alloca = nullptr,
-bool CastToDefaultAddrSpace = true);
+Address *Alloca = nullptr);
Address CreateMemTemp(QualType T, CharUnits Align, const Twine = "tmp",
-Address *Alloca = nullptr,
-bool CastToDefaultAddrSpace = true);
+Address *Alloca = nullptr);
+
+ /// CreateMemTemp - Create a temporary memory object of the given type, with
+ /// appropriate alignmen without casting it to the default address space.
+ Address CreateMemTempWithoutCast(QualType T, const Twine = "tmp");
+ Address CreateMemTempWithoutCast(QualType T, CharUnits Align,
+ const Twine = "tmp");
/// CreateAggTemp - Create a temporary memory object for the given
/// aggregate type.
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -61,11 +61,21 @@
/// CreateTempAlloca - This creates a alloca and inserts it into the entry
/// block.
+Address CodeGenFunction::CreateTempAllocaWithoutCast(llvm::Type *Ty,
+ CharUnits Align,
+ const Twine ,
+ llvm::Value *ArraySize) {
+ auto Alloca = CreateTempAlloca(Ty, Name, ArraySize);
+ Alloca->setAlignment(Align.getQuantity());
+ return Address(Alloca, Align);
+}
+
+/// CreateTempAlloca - This creates a alloca and inserts it into the entry
+/// block. The alloca is casted to default address space if necessary.
Address CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, CharUnits Align,
const Twine ,
llvm::Value *ArraySize,
- Address *AllocaAddr,
[PATCH] D47099: Disable casting of alloca for ActiveFlag
rjmccall added a comment. In https://reviews.llvm.org/D47099#1105574, @yaxunl wrote: > In https://reviews.llvm.org/D47099#1105493, @rjmccall wrote: > > > Maybe there should just be a method that makes a primitive alloca without > > the casting, and then you can call that in CreateTempAlloca. > > > In many cases we still need to call CreateTempAlloca with cast enabled, since > we are not certain there is only load from it and store to it. Any time it is > stored to another memory location or passed to another function (e.g. > constructor/destructor), it needs to be a pointer to the language's default > address space since the language sees it that way. Yes, I understand that. But there are some cases, like this, that do not need to produce something in `LangAS::Default`, and extracting out a method that just does an `alloca` without the unnecessary cast seems sensible. > An alternative fix would be just let ActiveFlag to be llvm::Value instead of > llvm::AllocaInst, since it does not really need to be an AllocaInst. I don't care at all about the type of `ActiveFlag`, but if we can generate better code at -O0 by avoiding the casts in situations where we obviously don't need them, we should do it. https://reviews.llvm.org/D47099 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47099: Disable casting of alloca for ActiveFlag
yaxunl added a comment. In https://reviews.llvm.org/D47099#1105493, @rjmccall wrote: > Maybe there should just be a method that makes a primitive alloca without the > casting, and then you can call that in CreateTempAlloca. In many cases we still need to call CreateTempAlloca with cast enabled, since we are not certain there is only load from it and store to it. Any time it is stored to another memory location or passed to another function (e.g. constructor/destructor), it needs to be a pointer to the language's default address space since the language sees it that way. https://reviews.llvm.org/D47099 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47099: Disable casting of alloca for ActiveFlag
rjmccall added a comment. Maybe there should just be a method that makes a primitive alloca without the casting, and then you can call that in CreateTempAlloca. https://reviews.llvm.org/D47099 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47099: Disable casting of alloca for ActiveFlag
yaxunl created this revision.
yaxunl added a reviewer: rjmccall.
ActiveFlag is a temporary variable emitted for clean up. It is defined as
AllocaInst* type and there is
a cast to AlllocaInst in SetActiveFlag. An alloca casted to generic pointer
causes assertion in
SetActiveFlag.
Since there is only load/store of ActiveFlag, it is safe to use the original
alloca, therefore disable
the cast.
https://reviews.llvm.org/D47099
Files:
lib/CodeGen/CGCleanup.cpp
test/CodeGenCXX/conditional-temporaries.cpp
Index: test/CodeGenCXX/conditional-temporaries.cpp
===
--- test/CodeGenCXX/conditional-temporaries.cpp
+++ test/CodeGenCXX/conditional-temporaries.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-apple-darwin9 -O3 |
FileCheck %s
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple=amdgcn-amd-amdhsa -O3 |
FileCheck %s
namespace {
Index: lib/CodeGen/CGCleanup.cpp
===
--- lib/CodeGen/CGCleanup.cpp
+++ lib/CodeGen/CGCleanup.cpp
@@ -284,7 +284,8 @@
void CodeGenFunction::initFullExprCleanup() {
// Create a variable to decide whether the cleanup needs to be run.
Address active = CreateTempAlloca(Builder.getInt1Ty(), CharUnits::One(),
-"cleanup.cond");
+"cleanup.cond", /*ArraySize=*/nullptr,
+/*Alloca=*/nullptr, /*Cast=*/false);
// Initialize it to false at a site that's guaranteed to be run
// before each evaluation.
Index: test/CodeGenCXX/conditional-temporaries.cpp
===
--- test/CodeGenCXX/conditional-temporaries.cpp
+++ test/CodeGenCXX/conditional-temporaries.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-apple-darwin9 -O3 | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple=amdgcn-amd-amdhsa -O3 | FileCheck %s
namespace {
Index: lib/CodeGen/CGCleanup.cpp
===
--- lib/CodeGen/CGCleanup.cpp
+++ lib/CodeGen/CGCleanup.cpp
@@ -284,7 +284,8 @@
void CodeGenFunction::initFullExprCleanup() {
// Create a variable to decide whether the cleanup needs to be run.
Address active = CreateTempAlloca(Builder.getInt1Ty(), CharUnits::One(),
-"cleanup.cond");
+"cleanup.cond", /*ArraySize=*/nullptr,
+/*Alloca=*/nullptr, /*Cast=*/false);
// Initialize it to false at a site that's guaranteed to be run
// before each evaluation.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
