ABataev added inline comments.
================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:63
+/// Common pre(post)-action for different OpenMP constructs.
+class CommonActionTy final : public PrePostActionTy {
+ llvm::Value *EnterCallee;
----------------
I don't think it is good to name it CommonActionTy, I think it is something
specific to NVPTX codegen. Could you give it some specific name + move it
somewhere to NVPTX-related part of code (to .cpp file, if possible)
================
Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:539-542
+ llvm::Value *EndArgs[] = {emitUpdateLocation(CGF, Loc), ThreadID};
+ CGF.EmitRuntimeCall(
+ createNVPTXRuntimeFunction(OMPRTL_NVPTX__kmpc_end_serialized_parallel),
+ EndArgs);
----------------
arpith-jacob wrote:
> ABataev wrote:
> > It is better to emit this code as PrePostAction, so it is called upon exit
> > of any cleanup scope
> Alexey, do you mean clean up during the execution of the serialized parallel
> region? Is something like this what you have in mind? Thanks.
>
> auto &&SeqGen = [this, Fn, &CapturedVars, &RTLoc, &Loc](CodeGenFunction
> &CGF,
> PrePostActionTy &) {
> auto &&CodeGen = [..](..) {
> llvm::Value *Args[] = {RTLoc, ThreadID};
> CGF.EmitRuntimeCall(
>
> createNVPTXRuntimeFunction(OMPRTL_NVPTX__kmpc_serialized_parallel),
> Args);
>
> llvm::SmallVector<llvm::Value *, 16> OutlinedFnArgs;
> OutlinedFnArgs.push_back(
> llvm::ConstantPointerNull::get(CGM.Int32Ty->getPointerTo()));
> OutlinedFnArgs.push_back(
> llvm::ConstantPointerNull::get(CGM.Int32Ty->getPointerTo()));
> OutlinedFnArgs.append(CapturedVars.begin(), CapturedVars.end());
> CGF.EmitCallOrInvoke(Fn, OutlinedFnArgs);
> };
>
> RegionCodeGenTy RCG(CodeGen);
> CommonActionTy Action(
> nullptr, llvm::None,
>
> createNVPTXRuntimeFunction(OMPRTL_NVPTX__kmpc_end_serialized_parallel),
> {emitUpdateLocation(CGF, Loc), ThreadID});
> RCG.setAction(Action);
> RCG(CGF);
> };
>
Yes, exactly.
================
Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:365
+ llvm::FunctionType *FnTy =
+ llvm::FunctionType::get(llvm::Type::getInt1Ty(CGM.getLLVMContext()),
+ TypeParams, /*isVarArg*/ false);
----------------
Does it really return I1 type? Or I8?
https://reviews.llvm.org/D28145
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
