From 39ec9de7c23063b87f5c56f4e80c8d0f8b511a4b Mon Sep 17 00:00:00 2001 From: Alex Voicu Date: Fri, 14 Feb 2025 11:20:45 +0000 Subject: [clang][CodeGen] `sret` args should always point to the `alloca` AS, so use that (#114062) `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). A target could still choose to do something different here, by e.g. overriding `classifyReturnType` behaviour. In a broader sense, this patch extends non-aliased indirect args to also carry an AS, which leads to changing the `getIndirect()` interface. At the moment we're only using this for (indirect) returns, but it allows for future handling of indirect args themselves. We default to using the AllocaAS as that matches what Clang is currently doing, however if, in the future, a target would opt for e.g. placing indirect returns in some other storage, with another AS, this will require revisiting. --------- Co-authored-by: Matt Arsenault Co-authored-by: Matt Arsenault --- clang/lib/CodeGen/CGExprAgg.cpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) (limited to 'clang/lib/CodeGen/CGExprAgg.cpp') diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp index c3f1cbed6b39..c574827ca094 100644 --- a/clang/lib/CodeGen/CGExprAgg.cpp +++ b/clang/lib/CodeGen/CGExprAgg.cpp @@ -296,18 +296,25 @@ void AggExprEmitter::withReturnValueSlot( (RequiresDestruction && Dest.isIgnored()); Address RetAddr = Address::invalid(); - RawAddress RetAllocaAddr = RawAddress::invalid(); EHScopeStack::stable_iterator LifetimeEndBlock; llvm::Value *LifetimeSizePtr = nullptr; llvm::IntrinsicInst *LifetimeStartInst = nullptr; if (!UseTemp) { - RetAddr = Dest.getAddress(); + // It is possible for the existing slot we are using directly to have been + // allocated in the correct AS for an indirect return, and then cast to + // the default AS (this is the behaviour of CreateMemTemp), however we know + // that the return address is expected to point to the uncasted AS, hence we + // strip possible pointer casts here. + if (Dest.getAddress().isValid()) + RetAddr = Dest.getAddress().withPointer( + Dest.getAddress().getBasePointer()->stripPointerCasts(), + Dest.getAddress().isKnownNonNull()); } else { - RetAddr = CGF.CreateMemTemp(RetTy, "tmp", &RetAllocaAddr); + RetAddr = CGF.CreateMemTempWithoutCast(RetTy, "tmp"); llvm::TypeSize Size = CGF.CGM.getDataLayout().getTypeAllocSize(CGF.ConvertTypeForMem(RetTy)); - LifetimeSizePtr = CGF.EmitLifetimeStart(Size, RetAllocaAddr.getPointer()); + LifetimeSizePtr = CGF.EmitLifetimeStart(Size, RetAddr.getBasePointer()); if (LifetimeSizePtr) { LifetimeStartInst = cast(std::prev(Builder.GetInsertPoint())); @@ -316,7 +323,7 @@ void AggExprEmitter::withReturnValueSlot( "Last insertion wasn't a lifetime.start?"); CGF.pushFullExprCleanup( - NormalEHLifetimeMarker, RetAllocaAddr, LifetimeSizePtr); + NormalEHLifetimeMarker, RetAddr, LifetimeSizePtr); LifetimeEndBlock = CGF.EHStack.stable_begin(); } } @@ -337,7 +344,7 @@ void AggExprEmitter::withReturnValueSlot( // Since we're not guaranteed to be in an ExprWithCleanups, clean up // eagerly. CGF.DeactivateCleanupBlock(LifetimeEndBlock, LifetimeStartInst); - CGF.EmitLifetimeEnd(LifetimeSizePtr, RetAllocaAddr.getPointer()); + CGF.EmitLifetimeEnd(LifetimeSizePtr, RetAddr.getBasePointer()); } } -- cgit v1.2.3