llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-flang-openmp Author: Jameson Nash (vtjnash) <details> <summary>Changes</summary> Careful type tracking here fixes 3 things: - Need for alloca getAllocatedType. - Need for GEP element type. - Incorrect use of a Bitcast followed by assert that it returned an Alloca. --- Patch is 27.07 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/181845.diff 4 Files Affected: - (modified) clang/lib/CodeGen/CGOpenMPRuntime.cpp (+16-16) - (modified) llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h (+25-14) - (modified) llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp (+92-114) - (modified) llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp (+21-20) ``````````diff diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp b/clang/lib/CodeGen/CGOpenMPRuntime.cpp index 3f3c239c2de78..f7cd349f79ebe 100644 --- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp +++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp @@ -10878,16 +10878,16 @@ static void emitTargetCallKernelLaunch( /*IsNonContiguous=*/true, /*ForEndCall=*/false); InputInfo.NumberOfTargetItems = Info.NumberOfPtrs; - InputInfo.BasePointersArray = Address(Info.RTArgs.BasePointersArray, + InputInfo.BasePointersArray = Address(Info.RTArgs.BasePointersArray.Ptr, CGF.VoidPtrTy, CGM.getPointerAlign()); - InputInfo.PointersArray = - Address(Info.RTArgs.PointersArray, CGF.VoidPtrTy, CGM.getPointerAlign()); + InputInfo.PointersArray = Address(Info.RTArgs.PointersArray.Ptr, + CGF.VoidPtrTy, CGM.getPointerAlign()); InputInfo.SizesArray = - Address(Info.RTArgs.SizesArray, CGF.Int64Ty, CGM.getPointerAlign()); - InputInfo.MappersArray = - Address(Info.RTArgs.MappersArray, CGF.VoidPtrTy, CGM.getPointerAlign()); - MapTypesArray = Info.RTArgs.MapTypesArray; - MapNamesArray = Info.RTArgs.MapNamesArray; + Address(Info.RTArgs.SizesArray.Ptr, CGF.Int64Ty, CGM.getPointerAlign()); + InputInfo.MappersArray = Address(Info.RTArgs.MappersArray.Ptr, CGF.VoidPtrTy, + CGM.getPointerAlign()); + MapTypesArray = Info.RTArgs.MapTypesArray.Ptr; + MapNamesArray = Info.RTArgs.MapNamesArray.Ptr; auto &&ThenGen = [&OMPRuntime, OutlinedFn, &D, &CapturedVars, RequiresOuterTask, &CS, OffloadingMandatory, Device, @@ -11806,16 +11806,16 @@ void CGOpenMPRuntime::emitTargetDataStandAloneCall( D.hasClausesOfKind<OMPNowaitClause>(); InputInfo.NumberOfTargetItems = Info.NumberOfPtrs; - InputInfo.BasePointersArray = Address(Info.RTArgs.BasePointersArray, + InputInfo.BasePointersArray = Address(Info.RTArgs.BasePointersArray.Ptr, CGF.VoidPtrTy, CGM.getPointerAlign()); - InputInfo.PointersArray = Address(Info.RTArgs.PointersArray, CGF.VoidPtrTy, - CGM.getPointerAlign()); + InputInfo.PointersArray = Address(Info.RTArgs.PointersArray.Ptr, + CGF.VoidPtrTy, CGM.getPointerAlign()); InputInfo.SizesArray = - Address(Info.RTArgs.SizesArray, CGF.Int64Ty, CGM.getPointerAlign()); - InputInfo.MappersArray = - Address(Info.RTArgs.MappersArray, CGF.VoidPtrTy, CGM.getPointerAlign()); - MapTypesArray = Info.RTArgs.MapTypesArray; - MapNamesArray = Info.RTArgs.MapNamesArray; + Address(Info.RTArgs.SizesArray.Ptr, CGF.Int64Ty, CGM.getPointerAlign()); + InputInfo.MappersArray = Address(Info.RTArgs.MappersArray.Ptr, + CGF.VoidPtrTy, CGM.getPointerAlign()); + MapTypesArray = Info.RTArgs.MapTypesArray.Ptr; + MapNamesArray = Info.RTArgs.MapNamesArray.Ptr; if (RequiresOuterTask) CGF.EmitOMPTargetTaskBasedDirective(D, ThenGen, InputInfo); else diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h index 9885ffc8b2065..3f9e6b2399d60 100644 --- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h +++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h @@ -2616,33 +2616,44 @@ class OpenMPIRBuilder { struct MapperAllocas &MapperAllocas, int64_t DeviceID, unsigned NumOperands); + /// A pointer value bundled with its array type, for typed offloading arrays. + /// \p t is the ArrayType of the alloca or GEP source; nullptr when the + /// pointer is a ConstantNull, a GlobalVariable, or otherwise type-unknown. + struct TypedPointerArray { + Value *Ptr = nullptr; + ArrayType *Ty = nullptr; + }; + /// Container for the arguments used to pass data to the runtime library. struct TargetDataRTArgs { /// The array of base pointer passed to the runtime library. - Value *BasePointersArray = nullptr; + TypedPointerArray BasePointersArray; /// The array of section pointers passed to the runtime library. - Value *PointersArray = nullptr; + TypedPointerArray PointersArray; /// The array of sizes passed to the runtime library. - Value *SizesArray = nullptr; + TypedPointerArray SizesArray; /// The array of map types passed to the runtime library for the beginning /// of the region or for the entire region if there are no separate map /// types for the region end. - Value *MapTypesArray = nullptr; + TypedPointerArray MapTypesArray; /// The array of map types passed to the runtime library for the end of the /// region, or nullptr if there are no separate map types for the region /// end. - Value *MapTypesArrayEnd = nullptr; + TypedPointerArray MapTypesArrayEnd; /// The array of user-defined mappers passed to the runtime library. - Value *MappersArray = nullptr; + TypedPointerArray MappersArray; /// The array of original declaration names of mapped pointers sent to the /// runtime library for debugging - Value *MapNamesArray = nullptr; + TypedPointerArray MapNamesArray; explicit TargetDataRTArgs() = default; - explicit TargetDataRTArgs(Value *BasePointersArray, Value *PointersArray, - Value *SizesArray, Value *MapTypesArray, - Value *MapTypesArrayEnd, Value *MappersArray, - Value *MapNamesArray) + explicit TargetDataRTArgs(TypedPointerArray BasePointersArray, + TypedPointerArray PointersArray, + TypedPointerArray SizesArray, + TypedPointerArray MapTypesArray, + TypedPointerArray MapTypesArrayEnd, + TypedPointerArray MappersArray, + TypedPointerArray MapNamesArray) : BasePointersArray(BasePointersArray), PointersArray(PointersArray), SizesArray(SizesArray), MapTypesArray(MapTypesArray), MapTypesArrayEnd(MapTypesArrayEnd), MappersArray(MappersArray), @@ -2769,9 +2780,9 @@ class OpenMPIRBuilder { } /// Return true if the current target data information has valid arrays. bool isValid() { - return RTArgs.BasePointersArray && RTArgs.PointersArray && - RTArgs.SizesArray && RTArgs.MapTypesArray && - (!HasMapper || RTArgs.MappersArray) && NumberOfPtrs; + return RTArgs.BasePointersArray.Ptr && RTArgs.PointersArray.Ptr && + RTArgs.SizesArray.Ptr && RTArgs.MapTypesArray.Ptr && + (!HasMapper || RTArgs.MappersArray.Ptr) && NumberOfPtrs; } bool requiresDevicePointerInfo() { return RequiresDevicePointerInfo; } bool separateBeginEndCalls() { return SeparateBeginEndCalls; } diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp index 25f4da7c90d90..850f90c0511ed 100644 --- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp +++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp @@ -564,12 +564,12 @@ void OpenMPIRBuilder::getKernelArgsVector(TargetKernelArgs &KernelArgs, ArgsVector = {Version, PointerNum, - KernelArgs.RTArgs.BasePointersArray, - KernelArgs.RTArgs.PointersArray, - KernelArgs.RTArgs.SizesArray, - KernelArgs.RTArgs.MapTypesArray, - KernelArgs.RTArgs.MapNamesArray, - KernelArgs.RTArgs.MappersArray, + KernelArgs.RTArgs.BasePointersArray.Ptr, + KernelArgs.RTArgs.PointersArray.Ptr, + KernelArgs.RTArgs.SizesArray.Ptr, + KernelArgs.RTArgs.MapTypesArray.Ptr, + KernelArgs.RTArgs.MapNamesArray.Ptr, + KernelArgs.RTArgs.MappersArray.Ptr, KernelArgs.NumIterations, Flags, NumTeams3D, @@ -8070,11 +8070,15 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createTargetData( } SmallVector<llvm::Value *, 13> OffloadingArgs = { - SrcLocInfo, DeviceID, - PointerNum, RTArgs.BasePointersArray, - RTArgs.PointersArray, RTArgs.SizesArray, - RTArgs.MapTypesArray, RTArgs.MapNamesArray, - RTArgs.MappersArray}; + SrcLocInfo, + DeviceID, + PointerNum, + RTArgs.BasePointersArray.Ptr, + RTArgs.PointersArray.Ptr, + RTArgs.SizesArray.Ptr, + RTArgs.MapTypesArray.Ptr, + RTArgs.MapNamesArray.Ptr, + RTArgs.MappersArray.Ptr}; if (IsStandAlone) { assert(MapperFunc && "MapperFunc missing for standalone target data"); @@ -8163,11 +8167,15 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createTargetData( SrcLocInfo = getOrCreateIdent(SrcLocStr, SrcLocStrSize); } - Value *OffloadingArgs[] = {SrcLocInfo, DeviceID, - PointerNum, RTArgs.BasePointersArray, - RTArgs.PointersArray, RTArgs.SizesArray, - RTArgs.MapTypesArray, RTArgs.MapNamesArray, - RTArgs.MappersArray}; + Value *OffloadingArgs[] = {SrcLocInfo, + DeviceID, + PointerNum, + RTArgs.BasePointersArray.Ptr, + RTArgs.PointersArray.Ptr, + RTArgs.SizesArray.Ptr, + RTArgs.MapTypesArray.Ptr, + RTArgs.MapNamesArray.Ptr, + RTArgs.MappersArray.Ptr}; Function *EndMapperFunc = getOrCreateRuntimeFunctionPtr(omp::OMPRTL___tgt_target_data_end_mapper); @@ -8671,16 +8679,6 @@ static Function *emitTargetTaskProxyFunction( Builder.CreateRetVoid(); return ProxyFn; } -static Type *getOffloadingArrayType(Value *V) { - - if (auto *GEP = dyn_cast<GetElementPtrInst>(V)) - return GEP->getSourceElementType(); - if (auto *Alloca = dyn_cast<AllocaInst>(V)) - return Alloca->getAllocatedType(); - - llvm_unreachable("Unhandled Instruction type"); - return nullptr; -} // This function returns a struct that has at most two members. // The first member is always %struct.kmp_task_ompbuilder_t, that is the task // descriptor. The second member, if needed, is a struct containing arrays @@ -8695,21 +8693,19 @@ static Type *getOffloadingArrayType(Value *V) { // %struct.task_with_privates is returned by this function. // If there aren't any offloading arrays to pass to the target kernel, // %struct.kmp_task_ompbuilder_t is returned. -static StructType * -createTaskWithPrivatesTy(OpenMPIRBuilder &OMPIRBuilder, - ArrayRef<Value *> OffloadingArraysToPrivatize) { +static StructType *createTaskWithPrivatesTy( + OpenMPIRBuilder &OMPIRBuilder, + ArrayRef<OpenMPIRBuilder::TypedPointerArray> OffloadingArraysToPrivatize) { if (OffloadingArraysToPrivatize.empty()) return OMPIRBuilder.Task; SmallVector<Type *, 4> StructFieldTypes; - for (Value *V : OffloadingArraysToPrivatize) { - assert(V->getType()->isPointerTy() && - "Expected pointer to array to privatize. Got a non-pointer value " - "instead"); - Type *ArrayTy = getOffloadingArrayType(V); - assert(ArrayTy && "ArrayType cannot be nullptr"); - StructFieldTypes.push_back(ArrayTy); + for (const OpenMPIRBuilder::TypedPointerArray &TPA : + OffloadingArraysToPrivatize) { + assert(TPA.Ty && + "ArrayType must be set for offloading arrays to privatize"); + StructFieldTypes.push_back(TPA.Ty); } StructType *PrivatesStructTy = StructType::create(StructFieldTypes, "struct.privates"); @@ -8901,16 +8897,16 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::emitTargetTask( emitBlock(OI.ExitBB, Builder.GetInsertBlock()->getParent(), /*IsFinished=*/true); - SmallVector<Value *, 2> OffloadingArraysToPrivatize; + SmallVector<TypedPointerArray, 2> OffloadingArraysToPrivatize; bool NeedsTargetTask = HasNoWait && DeviceID; if (NeedsTargetTask) { - for (auto *V : + for (auto TPA : {RTArgs.BasePointersArray, RTArgs.PointersArray, RTArgs.MappersArray, RTArgs.MapNamesArray, RTArgs.MapTypesArray, RTArgs.MapTypesArrayEnd, RTArgs.SizesArray}) { - if (V && !isa<ConstantPointerNull, GlobalVariable>(V)) { - OffloadingArraysToPrivatize.push_back(V); - OI.ExcludeArgsFromAggregate.push_back(V); + if (TPA.Ptr && !isa<ConstantPointerNull, GlobalVariable>(TPA.Ptr)) { + OffloadingArraysToPrivatize.push_back(TPA); + OI.ExcludeArgsFromAggregate.push_back(TPA.Ptr); } } } @@ -9042,20 +9038,12 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::emitTargetTask( Value *Privates = Builder.CreateStructGEP(TaskWithPrivatesTy, TaskData, 1); for (unsigned int i = 0; i < OffloadingArraysToPrivatize.size(); ++i) { - Value *PtrToPrivatize = OffloadingArraysToPrivatize[i]; - [[maybe_unused]] Type *ArrayType = - getOffloadingArrayType(PtrToPrivatize); - assert(ArrayType && "ArrayType cannot be nullptr"); - - Type *ElementType = PrivatesTy->getElementType(i); - assert(ElementType == ArrayType && - "ElementType should match ArrayType"); - (void)ArrayType; - + TypedPointerArray TPA = OffloadingArraysToPrivatize[i]; Value *Dst = Builder.CreateStructGEP(PrivatesTy, Privates, i); + assert(PrivatesTy->getElementType(i) == TPA.Ty); Builder.CreateMemCpy( - Dst, Alignment, PtrToPrivatize, Alignment, - Builder.getInt64(M.getDataLayout().getTypeStoreSize(ElementType))); + Dst, Alignment, TPA.Ptr, Alignment, + Builder.getInt64(M.getDataLayout().getTypeStoreSize(TPA.Ty))); } } @@ -9505,56 +9493,41 @@ void OpenMPIRBuilder::emitOffloadingArraysArgument(IRBuilderBase &Builder, bool ForEndCall) { assert((!ForEndCall || Info.separateBeginEndCalls()) && "expected region end call to runtime only when end call is separate"); - auto UnqualPtrTy = PointerType::getUnqual(M.getContext()); - auto VoidPtrTy = UnqualPtrTy; - auto VoidPtrPtrTy = UnqualPtrTy; + auto PtrTy = PointerType::getUnqual(M.getContext()); auto Int64Ty = Type::getInt64Ty(M.getContext()); - auto Int64PtrTy = UnqualPtrTy; if (!Info.NumberOfPtrs) { - RTArgs.BasePointersArray = ConstantPointerNull::get(VoidPtrPtrTy); - RTArgs.PointersArray = ConstantPointerNull::get(VoidPtrPtrTy); - RTArgs.SizesArray = ConstantPointerNull::get(Int64PtrTy); - RTArgs.MapTypesArray = ConstantPointerNull::get(Int64PtrTy); - RTArgs.MapNamesArray = ConstantPointerNull::get(VoidPtrPtrTy); - RTArgs.MappersArray = ConstantPointerNull::get(VoidPtrPtrTy); + RTArgs.BasePointersArray = {ConstantPointerNull::get(PtrTy)}; + RTArgs.PointersArray = {ConstantPointerNull::get(PtrTy)}; + RTArgs.SizesArray = {ConstantPointerNull::get(PtrTy)}; + RTArgs.MapTypesArray = {ConstantPointerNull::get(PtrTy)}; + RTArgs.MapNamesArray = {ConstantPointerNull::get(PtrTy)}; + RTArgs.MappersArray = {ConstantPointerNull::get(PtrTy)}; return; } - RTArgs.BasePointersArray = Builder.CreateConstInBoundsGEP2_32( - ArrayType::get(VoidPtrTy, Info.NumberOfPtrs), - Info.RTArgs.BasePointersArray, - /*Idx0=*/0, /*Idx1=*/0); - RTArgs.PointersArray = Builder.CreateConstInBoundsGEP2_32( - ArrayType::get(VoidPtrTy, Info.NumberOfPtrs), Info.RTArgs.PointersArray, - /*Idx0=*/0, - /*Idx1=*/0); - RTArgs.SizesArray = Builder.CreateConstInBoundsGEP2_32( - ArrayType::get(Int64Ty, Info.NumberOfPtrs), Info.RTArgs.SizesArray, - /*Idx0=*/0, /*Idx1=*/0); - RTArgs.MapTypesArray = Builder.CreateConstInBoundsGEP2_32( - ArrayType::get(Int64Ty, Info.NumberOfPtrs), - ForEndCall && Info.RTArgs.MapTypesArrayEnd ? Info.RTArgs.MapTypesArrayEnd - : Info.RTArgs.MapTypesArray, - /*Idx0=*/0, - /*Idx1=*/0); + ArrayType *ArrayPtrTy = ArrayType::get(PtrTy, Info.NumberOfPtrs); + RTArgs.BasePointersArray = {Info.RTArgs.BasePointersArray.Ptr, ArrayPtrTy}; + RTArgs.PointersArray = {Info.RTArgs.PointersArray.Ptr, ArrayPtrTy}; + RTArgs.SizesArray = {Info.RTArgs.SizesArray.Ptr, + ArrayType::get(Int64Ty, Info.NumberOfPtrs)}; + Value *MapTypes = ForEndCall && Info.RTArgs.MapTypesArrayEnd.Ptr + ? Info.RTArgs.MapTypesArrayEnd.Ptr + : Info.RTArgs.MapTypesArray.Ptr; + RTArgs.MapTypesArray = {MapTypes, ArrayType::get(Int64Ty, Info.NumberOfPtrs)}; // Only emit the mapper information arrays if debug information is // requested. if (!Info.EmitDebug) - RTArgs.MapNamesArray = ConstantPointerNull::get(VoidPtrPtrTy); + RTArgs.MapNamesArray = {ConstantPointerNull::get(PtrTy)}; else - RTArgs.MapNamesArray = Builder.CreateConstInBoundsGEP2_32( - ArrayType::get(VoidPtrTy, Info.NumberOfPtrs), Info.RTArgs.MapNamesArray, - /*Idx0=*/0, - /*Idx1=*/0); + RTArgs.MapNamesArray = {Info.RTArgs.MapNamesArray.Ptr, ArrayPtrTy}; // If there is no user-defined mapper, set the mapper array to nullptr to // avoid an unnecessary data privatization if (!Info.HasMapper) - RTArgs.MappersArray = ConstantPointerNull::get(VoidPtrPtrTy); + RTArgs.MappersArray = {ConstantPointerNull::get(PtrTy)}; else - RTArgs.MappersArray = - Builder.CreatePointerCast(Info.RTArgs.MappersArray, VoidPtrPtrTy); + RTArgs.MappersArray = {Info.RTArgs.MappersArray.Ptr, ArrayPtrTy}; } void OpenMPIRBuilder::emitNonContiguousDescriptor(InsertPointTy AllocaIP, @@ -9617,8 +9590,7 @@ void OpenMPIRBuilder::emitNonContiguousDescriptor(InsertPointTy AllocaIP, Value *DAddr = Builder.CreatePointerBitCastOrAddrSpaceCast( DimsAddr, Builder.getPtrTy()); Value *P = Builder.CreateConstInBoundsGEP2_32( - ArrayType::get(Builder.getPtrTy(), Info.NumberOfPtrs), - Info.RTArgs.PointersArray, 0, I); + Info.RTArgs.PointersArray.Ty, Info.RTArgs.PointersArray.Ptr, 0, I); Builder.CreateAlignedStore( DAddr, P, M.getDataLayout().getPrefTypeAlign(Builder.getPtrTy())); ++L; @@ -9928,14 +9900,18 @@ Error OpenMPIRBuilder::emitOffloadingArrays( ArrayType *PointerArrayType = ArrayType::get(Builder.getPtrTy(), Info.NumberOfPtrs); - Info.RTArgs.BasePointersArray = Builder.CreateAlloca( - PointerArrayType, /* ArraySize = */ nullptr, ".offload_baseptrs"); + Info.RTArgs.BasePointersArray = { + Builder.CreateAlloca(PointerArrayType, /* ArraySize = */ nullptr, + ".offload_baseptrs"), + PointerArrayType}; - Info.RTArgs.PointersArray = Builder.CreateAlloca( - PointerArrayType, /* ArraySize = */ nullptr, ".offload_ptrs"); + Info.RTArgs.PointersArray = {Builder.CreateAlloca(PointerArrayType, + /* ArraySize = */ nullptr, + ".offload_ptrs"), + PointerArrayType}; AllocaInst *MappersArray = Builder.CreateAlloca( PointerArrayType, /* ArraySize = */ nullptr, ".offload_mappers"); - Info.RTArgs.MappersArray = MappersArray; + Info.RTArgs.MappersArray = {MappersArray, PointerArrayType}; // If we don't have any VLA types or other types that require runtime // evaluation, we can use a constant array for the map sizes, otherwise we @@ -9963,8 +9939,10 @@ Error OpenMPIRBuilder::emitOffloadingArrays( if (RuntimeSizes.all()) { ArrayType *SizeArrayType = ArrayType::get(Int64Ty, Info.NumberOfPtrs); - Info.RTArgs.SizesArray = Builder.CreateAlloca( - SizeArrayType, /* ArraySize = */ nullptr, ".offload_sizes"); + Info.RTArgs.SizesArray = {Builder.CreateAlloca(SizeArrayType, + /* ArraySize = */ nullptr, + ".offload_sizes"), + ... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/181845 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
