[PATCH] D28125: [OpenMP] Update target codegen for NVPTX device.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:248
+  Address WorkFn = CGF.CreateTempAlloca(
+      CGF.Int8PtrTy, CharUnits::fromQuantity(8), /*Name*/ "work_fn");
+  Address ExecStatus =
----------------
I don't like the idea of using absolute numbers for alignment. Could use CreateDefaultAlignTempAlloca() instead?


================
Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:250-251
+  Address ExecStatus =
+      CGF.CreateTempAlloca(CGF.Int8Ty, CharUnits::fromQuantity(1),
+                           /*Name*/ "exec_status");
+  CGF.InitTempAlloca(ExecStatus, Bld.getInt8(/*C=*/0));
----------------
The same


================
Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:258-259
   // On termination condition (workid == 0), exit loop.
   llvm::Value *ShouldTerminate = Bld.CreateICmpEQ(
-      Bld.CreateAlignedLoad(WorkID, WorkID->getAlignment()),
-      llvm::Constant::getNullValue(WorkID->getType()->getElementType()),
+      Bld.CreateLoad(WorkFn), llvm::Constant::getNullValue(CGF.Int8PtrTy),
       "should_terminate");
----------------
Better to use Bld.CreateIsNull().


================
Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:266
+  llvm::Value *IsActive =
+      Bld.CreateICmpNE(Bld.CreateLoad(ExecStatus), Bld.getInt8(0), "is_active");
+  Bld.CreateCondBr(IsActive, ExecuteBB, BarrierBB);
----------------
Better to use Bld.CreateIsNotNull()


https://reviews.llvm.org/D28125