[PATCH] D11361: [OpenMP] Target directive host codegen
Samuel Antao via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 21 10:55:37 PDT 2015
sfantao added a comment.
In http://reviews.llvm.org/D11361#229744, @ABataev wrote:
> Another one thing I forget to mention. Current implementation of CGOpenMPRuntime is libomp-specific. You're trying to add functionality that is libtarget-specific. Maybe it is a good idea to separate support for libomp and libtarget runtime libraries?
Not sure what do you mean by separation. Different files? Different codegen class? My perspective is that the two things should be together given that they both address the same specification, and I see that interaction is required between the two components. E.g. teams codegen will have to interact with the target codegen (communicate number of teams/threads ) and the teams codegen will require the libomp interface in its implementation. We could always separate the two things in the future if we see that is a better way to organize the code.
================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:2887
@@ +2886,3 @@
+llvm::Value *
+CGOpenMPRuntime::emitTargetOutlinedFunction(CodeGenFunction &CGF,
+ const OMPExecutableDirective &D,
----------------
ABataev wrote:
> I don't think you need this argument. You're emitting a new outlined function here and don't need info about your current function.
Done!
================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:2906-2911
@@ +2905,8 @@
+
+ CodeGenFunction TargetAuxCGF(CGM, true);
+ CGOpenMPTargetRegionInfo CGInfo(CS, CodeGen);
+ CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(TargetAuxCGF, &CGInfo);
+ auto *TargetAuxFn = TargetAuxCGF.GenerateCapturedStmtFunction(CS);
+ TargetAuxFn->addFnAttr(llvm::Attribute::AlwaysInline);
+
+ // Collect the arguments of the main function.
----------------
ABataev wrote:
> You'd better to emit internal function separately in a new static function. Then you don't need to create TargetAuxCGF and TargetMainCGF. You should use just CGF everywhere. One CodeGenFunction instance per function.
Done!
================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:2970-2972
@@ +2969,5 @@
+
+ auto ai = Args.begin();
+ for (RecordDecl::field_iterator ri = RD->field_begin(), re = RD->field_end();
+ ri != re; ++ri, ++ai) {
+
----------------
ABataev wrote:
> Variable names should start with an upper case letter (e.g. Leader or Boats).
Ok, thought iterators were an exception to that rule. Fixed now!
================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:3070-3107
@@ +3069,40 @@
+ } else {
+ // We expect all the sizes to be constant, so we collect them to create
+ // a constant array.
+ SmallVector<uint64_t, 16> ConstSizes;
+ for (auto *V : Sizes)
+ ConstSizes.push_back(cast<llvm::ConstantInt>(V)->getZExtValue());
+
+ auto SizeTypeBytes =
+ CGF.getContext()
+ .getTypeSizeInChars(CGF.getContext().getSizeType())
+ .getQuantity();
+
+ llvm::Constant *SizesArrayInit;
+ switch (SizeTypeBytes) {
+ default:
+ llvm_unreachable("Unexpected size-type type!");
+ case 1: {
+ SmallVector<uint8_t, 16> ConstSizesL(ConstSizes.begin(),
+ ConstSizes.end());
+ SizesArrayInit =
+ llvm::ConstantDataArray::get(CGM.getLLVMContext(), ConstSizesL);
+ } break;
+ case 2: {
+ SmallVector<uint16_t, 16> ConstSizesL(ConstSizes.begin(),
+ ConstSizes.end());
+ SizesArrayInit =
+ llvm::ConstantDataArray::get(CGM.getLLVMContext(), ConstSizesL);
+ } break;
+ case 4: {
+ SmallVector<uint32_t, 16> ConstSizesL(ConstSizes.begin(),
+ ConstSizes.end());
+ SizesArrayInit =
+ llvm::ConstantDataArray::get(CGM.getLLVMContext(), ConstSizesL);
+ } break;
+ case 8: {
+ SizesArrayInit =
+ llvm::ConstantDataArray::get(CGM.getLLVMContext(), ConstSizes);
+ } break;
+ }
+ auto *SizesArrayGbl = new llvm::GlobalVariable(
----------------
ABataev wrote:
> Try instead:
> SizesArrayInit = llvm::ConstantArray::get(llvm::ArrayType::get(CGM.SizeTy, Sizes.size()), Sizes);
>
Done!
================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:3161-3164
@@ +3160,6 @@
+ } else {
+ BasePointersArray = llvm::Constant::getNullValue(CGM.VoidPtrPtrTy);
+ PointersArray = llvm::Constant::getNullValue(CGM.VoidPtrPtrTy);
+ SizesArray = llvm::Constant::getNullValue(CGM.SizeTy->getPointerTo());
+ MapTypesArray = llvm::Constant::getNullValue(CGM.Int32Ty->getPointerTo());
+ }
----------------
ABataev wrote:
> llvm::ConstantPointerNull::get(<type>);
Done!
================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:2139-2203
@@ +2138,67 @@
+
+ bool hasVLACaptures = false;
+ const CapturedStmt &CS = *cast<CapturedStmt>(S.getAssociatedStmt());
+ auto ri = CS.getCapturedRecordDecl()->field_begin();
+ auto ii = CS.capture_init_begin();
+ for (CapturedStmt::const_capture_iterator ci = CS.capture_begin(),
+ ce = CS.capture_end();
+ ci != ce; ++ci, ++ri, ++ii) {
+ StringRef Name;
+ QualType Ty;
+ llvm::Value *BasePointer;
+ llvm::Value *Pointer;
+ llvm::Value *Size;
+ unsigned MapType;
+
+ if (ci->capturesVariableArrayType()) {
+ llvm::Value *V = VLASizeMap[ri->getCapturedVLAType()->getSizeExpr()];
+ LValue LV = MakeNaturalAlignAddrLValue(
+ CreateMemTemp(ri->getType(), "__vla_size"), ri->getType());
+ EmitStoreThroughLValue(RValue::get(V), LV);
+ BasePointer = Pointer = LV.getAddress();
+ uint64_t SizeVal =
+ CGM.getContext().getTypeSizeInChars(ri->getType()).getQuantity();
+ Size = llvm::ConstantInt::get(CGM.SizeTy, SizeVal);
+
+ hasVLACaptures = true;
+ // VLA sizes don't need to be copied back from the device.
+ MapType = CGOpenMPRuntime::OMP_MAP_TO;
+ } else if (ci->capturesThis()) {
+ BasePointer = Pointer = LoadCXXThis();
+ const PointerType *PtrTy = cast<PointerType>(ri->getType().getTypePtr());
+ uint64_t SizeVal = CGM.getContext()
+ .getTypeSizeInChars(PtrTy->getPointeeType())
+ .getQuantity();
+ Size = llvm::ConstantInt::get(CGM.SizeTy, SizeVal);
+
+ // Default map type.
+ MapType = CGOpenMPRuntime::OMP_MAP_TO | CGOpenMPRuntime::OMP_MAP_FROM;
+ } else {
+ BasePointer = Pointer = EmitLValue(cast<DeclRefExpr>(*ii)).getAddress();
+
+ const ReferenceType *PtrTy =
+ cast<ReferenceType>(ri->getType().getTypePtr());
+ QualType ElementType = PtrTy->getPointeeType();
+
+ if (auto *VAT = dyn_cast<VariableArrayType>(ElementType.getTypePtr())) {
+ auto VATInfo = getVLASize(VAT);
+ Size = llvm::ConstantInt::get(
+ CGM.SizeTy,
+ CGM.getContext().getTypeSizeInChars(VATInfo.second).getQuantity());
+ Size = Builder.CreateNUWMul(Size, VATInfo.first);
+ } else {
+ uint64_t ElementTypeSize =
+ CGM.getContext().getTypeSizeInChars(ElementType).getQuantity();
+ Size = llvm::ConstantInt::get(CGM.SizeTy, ElementTypeSize);
+ }
+
+ // Default map type.
+ MapType = CGOpenMPRuntime::OMP_MAP_TO | CGOpenMPRuntime::OMP_MAP_FROM;
+ }
+
+ BasePointers.push_back(BasePointer);
+ Pointers.push_back(Pointer);
+ Sizes.push_back(Size);
+ MapTypes.push_back(MapType);
+ }
+
----------------
ABataev wrote:
> I think this one can be moved to runtime codegen
I moved this to codegen, but I still had to forward some VLA information that I can only get from CGF, so that the VLAMaps are exposed. I need that information so that I can initialize the target region parameters.
http://reviews.llvm.org/D11361
More information about the cfe-commits
mailing list