[SYCL] Option to disable alloca address space for sret arguments

clang/include/clang/Basic/CodeGenOptions.def

@@ -480,6 +480,9 @@ CODEGENOPT(DisableSYCLEarlyOpts, 1, 0)
 /// which do not contain "user" code.
 CODEGENOPT(OptimizeSYCLFramework, 1, 0)
 
+/// Whether to use alloca address space for `sret` arguments.
+CODEGENOPT(UseAllocaASForSrets, 1, 0)
+
 /// Turn on fp64 partial emulation for kernels with only fp64 conversion
 /// operations and no fp64 computation operations (requires Intel GPU backend
 /// supporting fp64 partial emulation)

clang/include/clang/Driver/Options.td

@@ -8827,6 +8827,11 @@ def fsycl_is_native_cpu : Flag<["-"], "fsycl-is-native-cpu">,
   HelpText<"Perform device compilation for Native CPU.">,
   Visibility<[CC1Option]>,
   MarshallingInfoFlag<LangOpts<"SYCLIsNativeCPU">>;
+defm offload_use_alloca_addrspace_for_srets : BoolFOption<"offload-use-alloca-addrspace-for-srets",
+  CodeGenOpts<"UseAllocaASForSrets">,
+  DefaultTrue,
+  PosFlag<SetTrue, [], [CC1Option], "Use alloca address space for sret arguments for offloading targets">,
+  NegFlag<SetFalse>>;
 
 } // let Visibility = [CC1Option]
 

clang/lib/CodeGen/ABIInfoImpl.cpp

@@ -21,10 +21,16 @@ ABIArgInfo DefaultABIInfo::classifyArgumentType(QualType Ty) const {
     // Records with non-trivial destructors/copy-constructors should not be
     // passed by value.
     if (CGCXXABI::RecordArgABI RAA = getRecordArgABI(Ty, getCXXABI()))
-      return getNaturalAlignIndirect(Ty, getDataLayout().getAllocaAddrSpace(),
+      return getNaturalAlignIndirect(Ty,
+                                     getCodeGenOpts().UseAllocaASForSrets
+                                         ? getDataLayout().getAllocaAddrSpace()
+                                         : CGT.getTargetAddressSpace(Ty),
                                      RAA == CGCXXABI::RAA_DirectInMemory);
 
-    return getNaturalAlignIndirect(Ty, getDataLayout().getAllocaAddrSpace());
+    return getNaturalAlignIndirect(Ty,
+                                   getCodeGenOpts().UseAllocaASForSrets
+                                       ? getDataLayout().getAllocaAddrSpace()
+                                       : CGT.getTargetAddressSpace(Ty));
   }
 
   // Treat an enum type as its underlying type.
@@ -37,7 +43,10 @@ ABIArgInfo DefaultABIInfo::classifyArgumentType(QualType Ty) const {
         Context.getTypeSize(Context.getTargetInfo().hasInt128Type()
                                 ? Context.Int128Ty
                                 : Context.LongLongTy))
-      return getNaturalAlignIndirect(Ty, getDataLayout().getAllocaAddrSpace());
+      return getNaturalAlignIndirect(Ty,
+                                     getCodeGenOpts().UseAllocaASForSrets
+                                         ? getDataLayout().getAllocaAddrSpace()
+                                         : CGT.getTargetAddressSpace(Ty));
 
   return (isPromotableIntegerTypeForABI(Ty)
               ? ABIArgInfo::getExtend(Ty, CGT.ConvertType(Ty))
@@ -49,7 +58,10 @@ ABIArgInfo DefaultABIInfo::classifyReturnType(QualType RetTy) const {
     return ABIArgInfo::getIgnore();
 
   if (isAggregateTypeForABI(RetTy))
-    return getNaturalAlignIndirect(RetTy, getDataLayout().getAllocaAddrSpace());
+    return getNaturalAlignIndirect(RetTy,
+                                   getCodeGenOpts().UseAllocaASForSrets
+                                       ? getDataLayout().getAllocaAddrSpace()
+                                       : CGT.getTargetAddressSpace(RetTy));
 
   // Treat an enum type as its underlying type.
   if (const EnumType *EnumTy = RetTy->getAs<EnumType>())
@@ -61,7 +73,9 @@ ABIArgInfo DefaultABIInfo::classifyReturnType(QualType RetTy) const {
                                      ? getContext().Int128Ty
                                      : getContext().LongLongTy))
       return getNaturalAlignIndirect(RetTy,
-                                     getDataLayout().getAllocaAddrSpace());
+                                     getCodeGenOpts().UseAllocaASForSrets
+                                         ? getDataLayout().getAllocaAddrSpace()
+                                         : CGT.getTargetAddressSpace(RetTy));
 
   return (isPromotableIntegerTypeForABI(RetTy) ? ABIArgInfo::getExtend(RetTy)
                                                : ABIArgInfo::getDirect());
@@ -122,14 +136,16 @@ CGCXXABI::RecordArgABI CodeGen::getRecordArgABI(QualType T, CGCXXABI &CXXABI) {
 }
 
 bool CodeGen::classifyReturnType(const CGCXXABI &CXXABI, CGFunctionInfo &FI,
-                                 const ABIInfo &Info) {
+                                 const ABIInfo &Info, CodeGenTypes &CGT) {
   QualType Ty = FI.getReturnType();
 
   if (const auto *RT = Ty->getAs<RecordType>())
     if (!isa<CXXRecordDecl>(RT->getDecl()) &&
         !RT->getDecl()->canPassInRegisters()) {
       FI.getReturnInfo() = Info.getNaturalAlignIndirect(
-          Ty, Info.getDataLayout().getAllocaAddrSpace());
+          Ty, Info.getCodeGenOpts().UseAllocaASForSrets
+                  ? Info.getDataLayout().getAllocaAddrSpace()
+                  : CGT.getTargetAddressSpace(Ty));
       return true;
     }
 

clang/lib/CodeGen/ABIInfoImpl.h

@@ -46,7 +46,7 @@ CGCXXABI::RecordArgABI getRecordArgABI(const RecordType *RT, CGCXXABI &CXXABI);
 CGCXXABI::RecordArgABI getRecordArgABI(QualType T, CGCXXABI &CXXABI);
 
 bool classifyReturnType(const CGCXXABI &CXXABI, CGFunctionInfo &FI,
-                        const ABIInfo &Info);
+                        const ABIInfo &Info, CodeGenTypes &CGT);
 
 /// Pass transparent unions as if they were the type of the first element. Sema
 /// should ensure that all elements of the union have the same "machine type".

clang/lib/CodeGen/CGCall.cpp

@@ -1719,8 +1719,12 @@ CodeGenTypes::GetFunctionType(const CGFunctionInfo &FI) {
 
   // Add type for sret argument.
   if (IRFunctionArgs.hasSRetArg()) {
-    ArgTypes[IRFunctionArgs.getSRetArgNo()] = llvm::PointerType::get(
-        getLLVMContext(), FI.getReturnInfo().getIndirectAddrSpace());
+    QualType Ret = FI.getReturnType();
+    unsigned AddressSpace = CGM.getCodeGenOpts().UseAllocaASForSrets
+                                ? FI.getReturnInfo().getIndirectAddrSpace()
+                                : CGM.getTypes().getTargetAddressSpace(Ret);
+    ArgTypes[IRFunctionArgs.getSRetArgNo()] =
+        llvm::PointerType::get(getLLVMContext(), AddressSpace);
   }
 
   // Add type for inalloca argument.
@@ -5309,6 +5313,7 @@ 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
@@ -5322,19 +5327,27 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
     } else if (!ReturnValue.isNull()) {
       SRetPtr = ReturnValue.getAddress();
     } else {
-      SRetPtr = CreateMemTempWithoutCast(RetTy, "tmp");
+      SRetPtr = CGM.getCodeGenOpts().UseAllocaASForSrets
+                    ? CreateMemTempWithoutCast(RetTy, "tmp")
+                    : CreateMemTemp(RetTy, "tmp", &SRetAlloca);
       if (HaveInsertPoint() && ReturnValue.isUnused()) {
         llvm::TypeSize size =
             CGM.getDataLayout().getTypeAllocSize(ConvertTypeForMem(RetTy));
-        UnusedReturnSizePtr = EmitLifetimeStart(size, SRetPtr.getBasePointer());
+        if (CGM.getCodeGenOpts().UseAllocaASForSrets)
+          UnusedReturnSizePtr =
+              EmitLifetimeStart(size, SRetPtr.getBasePointer());
+        else
+          UnusedReturnSizePtr =
+              EmitLifetimeStart(size, SRetAlloca.getPointer());
       }
     }
     if (IRFunctionArgs.hasSRetArg()) {
       // A mismatch between the allocated return value's AS and the target's
       // chosen IndirectAS can happen e.g. when passing the this pointer through
       // a chain involving stores to / loads from the DefaultAS; we address this
       // here, symmetrically with the handling we have for normal pointer args.
-      if (SRetPtr.getAddressSpace() != RetAI.getIndirectAddrSpace()) {
+      if (CGM.getCodeGenOpts().UseAllocaASForSrets &&
+          (SRetPtr.getAddressSpace() != RetAI.getIndirectAddrSpace())) {
         llvm::Value *V = SRetPtr.getBasePointer();
         LangAS SAS = getLangASFromTargetAS(SRetPtr.getAddressSpace());
         LangAS DAS = getLangASFromTargetAS(RetAI.getIndirectAddrSpace());
@@ -5916,9 +5929,14 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
   // can't depend on being inside of an ExprWithCleanups, so we need to manually
   // 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, SRetPtr,
-                                         UnusedReturnSizePtr);
+  if (UnusedReturnSizePtr) {
+    if (CGM.getCodeGenOpts().UseAllocaASForSrets)
+      pushFullExprCleanup<CallLifetimeEnd>(NormalEHLifetimeMarker, SRetPtr,
+                                           UnusedReturnSizePtr);
+    else
+      pushFullExprCleanup<CallLifetimeEnd>(NormalEHLifetimeMarker, SRetAlloca,
+                                           UnusedReturnSizePtr);
+  }
 
   llvm::BasicBlock *InvokeDest = CannotThrow ? nullptr : getInvokeDest();
 

clang/lib/CodeGen/ItaniumCXXABI.cpp

@@ -1349,10 +1349,14 @@ bool ItaniumCXXABI::classifyReturnType(CGFunctionInfo &FI) const {
 
   // If C++ prohibits us from making a copy, return by address.
   if (!RD->canPassInRegisters()) {
-    auto Align = CGM.getContext().getTypeAlignInChars(FI.getReturnType());
-    FI.getReturnInfo() = ABIArgInfo::getIndirect(
-        Align, /*AddrSpace=*/CGM.getDataLayout().getAllocaAddrSpace(),
-        /*ByVal=*/false);
+    QualType Ret = FI.getReturnType();
+    auto Align = CGM.getContext().getTypeAlignInChars(Ret);
+    unsigned AddressSpace = CGM.getCodeGenOpts().UseAllocaASForSrets
+                                ? CGM.getDataLayout().getAllocaAddrSpace()
+                                : CGM.getTypes().getTargetAddressSpace(Ret);
+    FI.getReturnInfo() =
+        ABIArgInfo::getIndirect(Align, /*AddrSpace=*/AddressSpace,
+                                /*ByVal=*/false);
     return true;
   }
   return false;

clang/lib/CodeGen/MicrosoftCXXABI.cpp

@@ -1172,10 +1172,14 @@ bool MicrosoftCXXABI::classifyReturnType(CGFunctionInfo &FI) const {
   bool isIndirectReturn = !isTrivialForABI || FI.isInstanceMethod();
 
   if (isIndirectReturn) {
-    CharUnits Align = CGM.getContext().getTypeAlignInChars(FI.getReturnType());
-    FI.getReturnInfo() = ABIArgInfo::getIndirect(
-        Align, /*AddrSpace=*/CGM.getDataLayout().getAllocaAddrSpace(),
-        /*ByVal=*/false);
+    QualType Ret = FI.getReturnType();
+    CharUnits Align = CGM.getContext().getTypeAlignInChars(Ret);
+    unsigned AddressSpace = CGM.getCodeGenOpts().UseAllocaASForSrets
+                                ? CGM.getDataLayout().getAllocaAddrSpace()
+                                : CGM.getTypes().getTargetAddressSpace(Ret);
+    FI.getReturnInfo() =
+        ABIArgInfo::getIndirect(Align, /*AddrSpace=*/AddressSpace,
+                                /*ByVal=*/false);
 
     // MSVC always passes `this` before the `sret` parameter.
     FI.getReturnInfo().setSRetAfterThis(FI.isInstanceMethod());

clang/lib/CodeGen/Targets/AArch64.cpp

@@ -60,7 +60,7 @@ class AArch64ABIInfo : public ABIInfo {
                    SmallVectorImpl<llvm::Type *> &Flattened) const;
 
   void computeInfo(CGFunctionInfo &FI) const override {
-    if (!::classifyReturnType(getCXXABI(), FI, *this))
+    if (!::classifyReturnType(getCXXABI(), FI, *this, CGT))
       FI.getReturnInfo() =
           classifyReturnType(FI.getReturnType(), FI.isVariadic());
 

clang/lib/CodeGen/Targets/ARM.cpp

@@ -232,7 +232,7 @@ void WindowsARMTargetCodeGenInfo::setTargetAttributes(
 }
 
 void ARMABIInfo::computeInfo(CGFunctionInfo &FI) const {
-  if (!::classifyReturnType(getCXXABI(), FI, *this))
+  if (!::classifyReturnType(getCXXABI(), FI, *this, CGT))
     FI.getReturnInfo() = classifyReturnType(FI.getReturnType(), FI.isVariadic(),
                                             FI.getCallingConvention());
 

clang/lib/CodeGen/Targets/SPIR.cpp

@@ -51,7 +51,10 @@ ABIArgInfo CommonSPIRABIInfo::classifyKernelArgumentType(QualType Ty) const {
     }
     // Pass all aggregate types allowed by Sema by value.
     if (isAggregateTypeForABI(Ty))
-      return getNaturalAlignIndirect(Ty, getDataLayout().getAllocaAddrSpace());
+      return getNaturalAlignIndirect(Ty,
+                                     getCodeGenOpts().UseAllocaASForSrets
+                                         ? getDataLayout().getAllocaAddrSpace()
+                                         : CGT.getTargetAddressSpace(Ty));
   }
 
   return DefaultABIInfo::classifyArgumentType(Ty);
@@ -129,7 +132,11 @@ ABIArgInfo CommonSPIRABIInfo::classifyRegcallArgumentType(QualType Ty) const {
     // Records with non-trivial destructors/copy-constructors should not be
     // passed by value.
     if (auto RAA = getRecordArgABI(Ty, getCXXABI()))
-      return getNaturalAlignIndirect(Ty, RAA == CGCXXABI::RAA_DirectInMemory);
+      return getNaturalAlignIndirect(Ty,
+                                     getCodeGenOpts().UseAllocaASForSrets
+                                         ? getDataLayout().getAllocaAddrSpace()
+                                         : CGT.getTargetAddressSpace(Ty),
+                                     RAA == CGCXXABI::RAA_DirectInMemory);
 
     // Ignore empty structs/unions.
     if (isEmptyRecord(getContext(), Ty, true))
@@ -321,7 +328,10 @@ ABIArgInfo SPIRVABIInfo::classifyArgumentType(QualType Ty) const {
   // Records with non-trivial destructors/copy-constructors should not be
   // passed by value.
   if (auto RAA = getRecordArgABI(Ty, getCXXABI()))
-    return getNaturalAlignIndirect(Ty, getDataLayout().getAllocaAddrSpace(),
+    return getNaturalAlignIndirect(Ty,
+                                   getCodeGenOpts().UseAllocaASForSrets
+                                       ? getDataLayout().getAllocaAddrSpace()
+                                       : CGT.getTargetAddressSpace(Ty),
                                    RAA == CGCXXABI::RAA_DirectInMemory);
 
   if (const RecordType *RT = Ty->getAs<RecordType>()) {

clang/lib/CodeGen/Targets/X86.cpp

@@ -970,7 +970,7 @@ void X86_32ABIInfo::computeInfo(CGFunctionInfo &FI) const {
   } else
     State.FreeRegs = DefaultNumRegisterParameters;
 
-  if (!::classifyReturnType(getCXXABI(), FI, *this)) {
+  if (!::classifyReturnType(getCXXABI(), FI, *this, CGT)) {
     FI.getReturnInfo() = classifyReturnType(FI.getReturnType(), State);
   } else if (FI.getReturnInfo().isIndirect()) {
     // The C++ ABI is not aware of register usage, so we have to check if the
@@ -2980,7 +2980,7 @@ void X86_64ABIInfo::computeInfo(CGFunctionInfo &FI) const {
   unsigned FreeSSERegs = IsRegCall ? 16 : 8;
   unsigned NeededInt = 0, NeededSSE = 0, MaxVectorWidth = 0;
 
-  if (!::classifyReturnType(getCXXABI(), FI, *this)) {
+  if (!::classifyReturnType(getCXXABI(), FI, *this, CGT)) {
     if (IsRegCall && FI.getReturnType()->getTypePtr()->isRecordType() &&
         !FI.getReturnType()->getTypePtr()->isUnionType()) {
       FI.getReturnInfo() = classifyRegCallStructType(

clang/lib/Driver/ToolChains/Clang.cpp

@@ -5811,6 +5811,11 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
 
         CmdArgs.push_back("-fsycl-optimize-non-user-code");
       }
+
+      // Disable this option for SYCL by default.
+      // TODO:  This needs to be re-enabled once we have a real fix.
+      CmdArgs.push_back("-fno-offload-use-alloca-addrspace-for-srets");
+
       // Add any predefined macros associated with intel_gpu* type targets
       // passed in with -fsycl-targets
       // TODO: Macros are populated during device compilations and saved for
@@ -6364,6 +6369,10 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
     Args.addOptOutFlag(CmdArgs, options::OPT_foptimize_sibling_calls,
                        options::OPT_fno_optimize_sibling_calls);
 
+    Args.addOptOutFlag(CmdArgs,
+                       options::OPT_foffload_use_alloca_addrspace_for_srets,
+                       options::OPT_fno_offload_use_alloca_addrspace_for_srets);
+
     RenderFloatingPointOptions(TC, D, isOptimizationLevelFast(Args), Args,
                                CmdArgs, JA, NoOffloadFP32PrecDiv,
                                NoOffloadFP32PrecSqrt);

clang/test/CodeGenSYCL/regcall-cc-test.cpp

@@ -333,7 +333,7 @@ struct NonCopyable {
 // CHECK-DAG: %struct.NonCopyable = type { i32 }
 
 SYCL_DEVICE int __regcall bar(NonCopyable x) {
-// CHECK-DAG: define dso_local x86_regcallcc noundef i32 @_Z15__regcall3__bar11NonCopyable(ptr noundef byval(%struct.NonCopyable) align 4 %x)
+// CHECK-DAG: define dso_local x86_regcallcc noundef i32 @_Z15__regcall3__bar11NonCopyable(ptr noundef %x)
   return x.a;
 }
 

clang/test/Driver/sycl-device.cpp

@@ -58,3 +58,15 @@
 // PHASES-PREPROC-DEPS: 0: input, {{.*}}, c++, (device-sycl)
 // PHASES-PROPROC-DEPS: 1: preprocessor, {0}, dependencies, (device-sycl)
 // PHASES-PREPROC-DEPS: 2: offload, "device-sycl (spir64-unknown-unknown)" {1}, none
+
+/// Check that "-fno-offload-use-alloca-addrspace-for-srets" is not set by
+/// default on the command-line in a non-sycl compilation.
+// RUN:   %clang -### %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ALLOCA-ADDRSPACE %s
+// CHECK-ALLOCA-ADDRSPACE-NOT: clang{{.*}} "-fno-offload-use-alloca-addrspace-for-srets"
+
+/// Check that "-fno-offload-use-alloca-addrspace-for-srets" is set if it is
+/// not specified on the command-line by the user with -fsycl
+// RUN:   %clang -### -fsycl %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-NO-ALLOCA-ADDRSPACE %s
+// CHECK-NO-ALLOCA-ADDRSPACE: clang{{.*}} "-fno-offload-use-alloca-addrspace-for-srets"

clang/test/Driver/sycl-int-footer-old-model.cpp

@@ -1,7 +1,7 @@
 /// Check compilation tool steps when using the integration footer
 // RUN:  %clangxx -fsycl --no-offload-new-driver -I cmdline/dir -include dummy.h %/s -### 2>&1 \
 // RUN:   | FileCheck -check-prefix FOOTER %s -DSRCDIR=%/S -DCMDDIR=cmdline/dir
-// FOOTER: clang{{.*}} "-fsycl-is-device"{{.*}} "-fsycl-int-header=[[INTHEADER:.+\.h]]" "-fsycl-int-footer=[[INTFOOTER:.+\h]]" "-sycl-std={{.*}}"{{.*}} "-include" "dummy.h"
+// FOOTER: clang{{.*}} "-fsycl-is-device"{{.*}} "-fsycl-int-header=[[INTHEADER:.+\.h]]" "-fsycl-int-footer=[[INTFOOTER:.+\h]]"{{.*}} "-sycl-std={{.*}}"{{.*}} "-include" "dummy.h"
 // FOOTER: clang{{.*}} "-fsycl-is-host"
 // FOOTER-SAME: "-include-internal-header" "[[INTHEADER]]"
 // FOOTER-SAME: "-dependency-filter" "[[INTHEADER]]"
@@ -10,7 +10,7 @@
 /// Preprocessed file creation with integration footer
 // RUN: %clangxx -fsycl --no-offload-new-driver -E %/s -### 2>&1 \
 // RUN:   | FileCheck -check-prefix FOOTER_PREPROC_GEN %s
-// FOOTER_PREPROC_GEN: clang{{.*}} "-fsycl-is-device"{{.*}} "-fsycl-int-header=[[INTHEADER:.+\.h]]" "-fsycl-int-footer=[[INTFOOTER:.+\h]]" "-sycl-std={{.*}}" "-o" "[[PREPROC_DEVICE:.+\.ii]]"
+// FOOTER_PREPROC_GEN: clang{{.*}} "-fsycl-is-device"{{.*}} "-fsycl-int-header=[[INTHEADER:.+\.h]]" "-fsycl-int-footer=[[INTFOOTER:.+\h]]"{{.*}} "-sycl-std={{.*}}" "-o" "[[PREPROC_DEVICE:.+\.ii]]"
 // FOOTER_PREPROC_GEN: clang{{.*}} "-fsycl-is-host"
 // FOOTER_PREPROC_GEN-SAME: "-include-internal-header" "[[INTHEADER]]"
 // FOOTER_PREPROC_GEN-SAME: "-dependency-filter" "[[INTHEADER]]"
@@ -29,7 +29,7 @@
 /// Check that integration footer can be disabled
 // RUN:  %clangxx -fsycl --no-offload-new-driver -fno-sycl-use-footer %s -### 2>&1 \
 // RUN:   | FileCheck -check-prefix NO-FOOTER --implicit-check-not "-fsycl-int-footer" %s
-// NO-FOOTER: clang{{.*}} "-fsycl-is-device"{{.*}} "-fsycl-int-header=[[INTHEADER:.+\.h]]" "-sycl-std={{.*}}"
+// NO-FOOTER: clang{{.*}} "-fsycl-is-device"{{.*}} "-fsycl-int-header=[[INTHEADER:.+\.h]]"{{.*}} "-sycl-std={{.*}}"
 // NO-FOOTER-NOT: append-file
 // NO-FOOTER: clang{{.*}} "-fsycl-is-host"{{.*}} "-include-internal-header" "[[INTHEADER]]"