[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