[PATCH] D11361: [OpenMP] Target directive host codegen
Alexey Bataev via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 21 01:29:51 PDT 2015
ABataev added inline comments.
================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:2887
@@ +2886,3 @@
+llvm::Value *
+CGOpenMPRuntime::emitTargetOutlinedFunction(CodeGenFunction &CGF,
+ const OMPExecutableDirective &D,
----------------
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.
================
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.
----------------
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.
================
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) {
+
----------------
Variable names should start with an upper case letter (e.g. Leader or Boats).
================
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(
----------------
Try instead:
SizesArrayInit = llvm::ConstantArray::get(llvm::ArrayType::get(CGM.SizeTy, Sizes.size()), Sizes);
================
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());
+ }
----------------
llvm::ConstantPointerNull::get(<type>);
================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:190-204
@@ +189,17 @@
+public:
+ /// \brief Values for bit flags used to specify the mapping type for
+ /// offloading.
+ enum OpenMPOffloadMappingFlags {
+ /// \brief Allocate memory on the device and move data from host to device.
+ OMP_MAP_TO = 0x01,
+ /// \brief Allocate memory on the device and move data from device to host.
+ OMP_MAP_FROM = 0x02,
+ };
+
+private:
+ enum OpenMPOffloadingReservedDeviceIDs {
+ /// \brief Device ID if the device was not defined, runtime should get it
+ /// from environment variables in the spec.
+ OMP_DEVICEID_UNDEF = -1,
+ };
+
----------------
These enums must not be exposed by CGOpenMPRuntime until they are used in arguments of runtime member functions.
================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:768
@@ -710,1 +767,3 @@
+ ArrayRef<llvm::Value *> Sizes,
+ ArrayRef<unsigned> MapTypes, bool hasVLACaptures);
};
----------------
I don't like the idea of using 'unsigned' as a map type. You should create some particular OpenMPMapClauseKind (just like OpenMPDefaultClauseKind, OpenMPDependClauseKind, OpenMPProcBindClauseKind etc.) and use it where required.
================
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);
+ }
+
----------------
I think this one can be moved to runtime codegen
http://reviews.llvm.org/D11361
More information about the cfe-commits
mailing list