ABataev added a comment.
The patch is too big and quite hard to review? Could you split it into several
smaller parts?
================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:4280-4282
+ // We don't need debug information in this function as nothing here refers to
+ // user source code.
+ CGF.disableDebugInfo();
----------------
It is not quite so, at least we have a reference in a list of reductions in
`reduction` clause, which may be considered a debug position
================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:524
+
+ static bool classof(const CGOpenMPRuntime *RT) {
+ return RT->getKind() == RK_HOST;
----------------
arpith-jacob wrote:
> This is required to cast to the NVPTX runtime in a static function as follows;
>
> CGOpenMPRuntimeNVPTX &RT = cast<CGOpenMPRuntimeNVPTX>(CGM.getOpenMPRuntime());
Are you going to make calls to `isa()`, `dyn_cast()` functions? If not, just
use `static_cast<>()` instead.
================
Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:972-973
+ auto CastTy = Size <= 4 ? CGM.Int32Ty : CGM.Int64Ty;
+ auto *ElemCast = Bld.CreateSExtOrBitCast(Elem, CastTy);
+ auto *WarpSize = Bld.CreateTruncOrBitCast(getNVPTXWarpSize(CGF),
CGM.Int16Ty);
+
----------------
I'd prefer you to use `CGF.EmitScalarConversion()` rather than Builder casts.
================
Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:975-978
+ llvm::SmallVector<llvm::Value *, 3> Args;
+ Args.push_back(ElemCast);
+ Args.push_back(Offset);
+ Args.push_back(WarpSize);
----------------
Do you really need a SmallVector<> or you can use just an array here?
================
Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:1140-1141
+ if (ShuffleInElement)
+ Elem = createRuntimeShuffleFunction(CGF, Private->getType(), Elem,
+ RemoteLaneOffset);
+
----------------
Enclose in braces
================
Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:1153-1156
+ CGF.EmitStoreOfScalar(Bld.CreatePointerBitCastOrAddrSpaceCast(
+ DestElementAddr.getPointer(), CGF.VoidPtrTy),
+ DestElementPtrAddr, /*Volatile=*/false,
+ C.VoidPtrTy);
----------------
Braces
================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:2249
+ ? OMPD_parallel_for_simd
+ : OMPD_parallel);
// Emit post-update of the reduction variables if IsLastIter != 0.
----------------
OMPD_parallel or OMPD_parallel_for?
================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:2424
CGF.OMPCancelStack.emitExit(CGF, S.getDirectiveKind(), CodeGen);
- CGF.EmitOMPReductionClauseFinal(S);
+ CGF.EmitOMPReductionClauseFinal(S, OMPD_parallel);
// Emit post-update of the reduction variables if IsLastIter != 0.
----------------
OMPD_parallel_for?
https://reviews.llvm.org/D29506
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
