[PATCH] D17877: [OpenMP] Base support for target directive codegen on NVPTX device.
Alexey Bataev via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 8 22:32:40 PST 2016
ABataev added inline comments.
================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:69
@@ +68,3 @@
+ /// evaluates to false.
+ void emitTargetOutlinedFunctionHelper(const OMPExecutableDirective &D,
+ StringRef ParentName,
----------------
Missed 'virtual'
================
Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:27
@@ +26,3 @@
+ &CGM.getModule(), llvm::Intrinsic::nvvm_read_ptx_sreg_warpsize),
+ {}, "nvptx_warp_size");
+}
----------------
Use 'llvm::None' instead of '{}'
================
Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:36
@@ +35,3 @@
+ &CGM.getModule(), llvm::Intrinsic::nvvm_read_ptx_sreg_tid_x),
+ {}, "nvptx_tid");
+}
----------------
Use 'llvm::None' instead of '{}'
================
Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:45
@@ +44,3 @@
+ &CGM.getModule(), llvm::Intrinsic::nvvm_read_ptx_sreg_ntid_x),
+ {}, "nvptx_num_threads");
+}
----------------
Use 'llvm::None' instead of '{}'
================
Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:53
@@ +52,3 @@
+ &CGM.getModule(), llvm::Intrinsic::nvvm_barrier0),
+ {});
+}
----------------
Remove '{}', not required
================
Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:90
@@ +89,3 @@
+};
+} // anonymous namespace
+
----------------
Just 'namespace', remove 'anonymous'
================
Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:104
@@ +103,3 @@
+ FunctionType::ExtInfo EI;
+ CGFI = &CGM.getTypes().arrangeFreeFunctionDeclaration(Ctx.VoidTy, {}, EI,
+ /*isVariadic=*/false);
----------------
Use '.arrangeNullaryFunction()' instead
================
Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:235
@@ +234,3 @@
+ CGF.EmitBlock(WorkerBB);
+ CGF.EmitCallOrInvoke(WST.WorkerFn, {});
+ CGF.EmitBranch(EST.ExitBB);
----------------
Use 'llvm::None' as the second arg
================
Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.h:26
@@ -22,2 +25,3 @@
class CGOpenMPRuntimeNVPTX : public CGOpenMPRuntime {
+ //
----------------
All functions must start with a lower case letter
http://reviews.llvm.org/D17877
More information about the cfe-commits
mailing list