llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Alex Voicu (AlexVlx) <details> <summary>Changes</summary> `sret` arguments are always going to reside in the stack/`alloca` address space, which makes the current formulation where their AS is derived from the pointee somewhat quaint. This patch ensures that `sret` ends up pointing to the `alloca` AS in IR function signatures, and also guards agains trying to pass a casted `alloca`d pointer to a `sret` arg, which can happen for most languages, when compiled for targets that have a non-zero `alloca` AS (e.g. AMDGCN) / map `LangAS::default` to a non-zero value (SPIR-V). --- Full diff: https://github.com/llvm/llvm-project/pull/114062.diff 3 Files Affected: - (modified) clang/lib/CodeGen/CGCall.cpp (+8-7) - (modified) clang/test/CodeGen/partial-reinitialization2.c (+2-2) - (modified) clang/test/CodeGen/sret.c (+11) ``````````diff diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index 8f4f5d3ed81601..56acfae7ae9e51 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -1672,8 +1672,7 @@ CodeGenTypes::GetFunctionType(const CGFunctionInfo &FI) { // Add type for sret argument. if (IRFunctionArgs.hasSRetArg()) { - QualType Ret = FI.getReturnType(); - unsigned AddressSpace = CGM.getTypes().getTargetAddressSpace(Ret); + unsigned AddressSpace = CGM.getDataLayout().getAllocaAddrSpace(); ArgTypes[IRFunctionArgs.getSRetArgNo()] = llvm::PointerType::get(getLLVMContext(), AddressSpace); } @@ -5145,7 +5144,6 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, // If the call returns a temporary with struct return, create a temporary // alloca to hold the result, unless one is given to us. Address SRetPtr = Address::invalid(); - RawAddress SRetAlloca = RawAddress::invalid(); llvm::Value *UnusedReturnSizePtr = nullptr; if (RetAI.isIndirect() || RetAI.isInAlloca() || RetAI.isCoerceAndExpand()) { // For virtual function pointer thunks and musttail calls, we must always @@ -5159,16 +5157,19 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, } else if (!ReturnValue.isNull()) { SRetPtr = ReturnValue.getAddress(); } else { - SRetPtr = CreateMemTemp(RetTy, "tmp", &SRetAlloca); + SRetPtr = CreateMemTempWithoutCast(RetTy, "tmp"); if (HaveInsertPoint() && ReturnValue.isUnused()) { llvm::TypeSize size = CGM.getDataLayout().getTypeAllocSize(ConvertTypeForMem(RetTy)); - UnusedReturnSizePtr = EmitLifetimeStart(size, SRetAlloca.getPointer()); + UnusedReturnSizePtr = EmitLifetimeStart(size, SRetPtr.getBasePointer()); } } if (IRFunctionArgs.hasSRetArg()) { + // If the caller allocated the return slot, it is possible that the + // alloca was AS casted to the default as, so we ensure the cast is + // stripped before binding to the sret arg, which is in the allocaAS. IRCallArgs[IRFunctionArgs.getSRetArgNo()] = - getAsNaturalPointerTo(SRetPtr, RetTy); + getAsNaturalPointerTo(SRetPtr, RetTy)->stripPointerCasts(); } else if (RetAI.isInAlloca()) { Address Addr = Builder.CreateStructGEP(ArgMemory, RetAI.getInAllocaFieldIndex()); @@ -5740,7 +5741,7 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, // pop this cleanup later on. Being eager about this is OK, since this // temporary is 'invisible' outside of the callee. if (UnusedReturnSizePtr) - pushFullExprCleanup<CallLifetimeEnd>(NormalEHLifetimeMarker, SRetAlloca, + pushFullExprCleanup<CallLifetimeEnd>(NormalEHLifetimeMarker, SRetPtr, UnusedReturnSizePtr); llvm::BasicBlock *InvokeDest = CannotThrow ? nullptr : getInvokeDest(); diff --git a/clang/test/CodeGen/partial-reinitialization2.c b/clang/test/CodeGen/partial-reinitialization2.c index e709c1d4ad1ee1..7949a69555031e 100644 --- a/clang/test/CodeGen/partial-reinitialization2.c +++ b/clang/test/CodeGen/partial-reinitialization2.c @@ -91,8 +91,8 @@ void test5(void) // CHECK-LABEL: test6 void test6(void) { - // CHECK: [[LP:%[a-z0-9]+]] = getelementptr{{.*}}%struct.LLP2P2, ptr{{.*}}, i32 0, i32 0 - // CHECK: call {{.*}}get456789(ptr {{.*}}[[LP]]) + // CHECK: [[VAR:%[a-z0-9]+]] = alloca + // CHECK: call {{.*}}get456789(ptr {{.*}}sret{{.*}} [[VAR]]) // CHECK: [[CALL:%[a-z0-9]+]] = call {{.*}}@get235() // CHECK: store{{.*}}[[CALL]], {{.*}}[[TMP0:%[a-z0-9.]+]] diff --git a/clang/test/CodeGen/sret.c b/clang/test/CodeGen/sret.c index 6d905e89b2c6fd..3b4914f29d2bfe 100644 --- a/clang/test/CodeGen/sret.c +++ b/clang/test/CodeGen/sret.c @@ -1,4 +1,5 @@ // RUN: %clang_cc1 %s -Wno-strict-prototypes -emit-llvm -o - | FileCheck %s +// RUN: %clang_cc1 %s -Wno-strict-prototypes -triple amdgcn-amd-amdhsa -emit-llvm -o - | FileCheck --check-prefix=NONZEROALLOCAAS %s struct abc { long a; @@ -6,18 +7,28 @@ struct abc { long c; long d; long e; + long f; + long g; + long h; + long i; + long j; }; struct abc foo1(void); // CHECK-DAG: declare {{.*}} @foo1(ptr dead_on_unwind writable sret(%struct.abc) +// NONZEROALLOCAAS-DAG: declare {{.*}} @foo1(ptr addrspace(5) dead_on_unwind writable sret(%struct.abc) struct abc foo2(); // CHECK-DAG: declare {{.*}} @foo2(ptr dead_on_unwind writable sret(%struct.abc) +// NONZEROALLOCAAS-DAG: declare {{.*}} @foo2(ptr addrspace(5) dead_on_unwind writable sret(%struct.abc) struct abc foo3(void){} // CHECK-DAG: define {{.*}} @foo3(ptr dead_on_unwind noalias writable sret(%struct.abc) +// NONZEROALLOCAAS-DAG: define {{.*}} @foo3(ptr addrspace(5) dead_on_unwind noalias writable sret(%struct.abc) void bar(void) { struct abc dummy1 = foo1(); // CHECK-DAG: call {{.*}} @foo1(ptr dead_on_unwind writable sret(%struct.abc) + // NONZEROALLOCAAS-DAG: call {{.*}} @foo1(ptr addrspace(5) dead_on_unwind writable sret(%struct.abc) struct abc dummy2 = foo2(); // CHECK-DAG: call {{.*}} @foo2(ptr dead_on_unwind writable sret(%struct.abc) + // NONZEROALLOCAAS-DAG: call {{.*}} @foo2(ptr addrspace(5) dead_on_unwind writable sret(%struct.abc) } `````````` </details> https://github.com/llvm/llvm-project/pull/114062 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
