[PATCH] D133120: [OpenMP][Opt] Fix target region without parallel

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

LG, minor nits only. I assume you can address them before committing, let me know if there is an issue. Thanks for fixing this properly.



================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:3674
+      BranchInst::Create(ReturnBB, UserCodeBB, IsMainThread, InitBB);
+    }
 
----------------
Style: Given that we have now a conditional, let's move the two long code sequences in helper methods. It's hard to read like this as the "else" is easily over looked (by me).

Remove all the `llvm::` above.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:4153
         !mayContainParallelRegion())
       SPMDCompatibilityTracker.indicatePessimisticFixpoint();
 
----------------
The above conditional should be deleted. This might cause more tests to be affected.


================
Comment at: llvm/test/Transforms/OpenMP/get_hardware_num_threads_in_block_fold.ll:23
+
+; This is a target region without a parallel region so it is SPMD but uses just 1 thread per team
+; CHECK-NEXT:    [[THREAD_ID_IN_BLOCK:%.*]] = call i32 @__kmpc_get_hardware_thread_id_in_block()
----------------
Put the comments before the function so they are not deleted when we auto-generate the check lines. Also, auto-generate the check lines ;)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133120/new/

https://reviews.llvm.org/D133120