[PATCH] D114331: [OMPIRBuilder] Fix static schedule in worksharing-loop directive

[Peixin Qiao via Phabricator via llvm-commits]

peixin added inline comments.


================
Comment at: mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:726
+    // The chunk size defaults to 1 when it is unspecified.
+    if (!chunk)
+      chunk = llvm::ConstantInt::get(ivType, 1);
----------------
Leporacanthicus wrote:
> I may be a bit confused here, but isn't this setting `chunk` to not-null, and then expecting the apply function to see it as null? 
This code is the "else" branch, which is for non-static schedule. As OpenMP 5.1 [2.11.4] states, the chunk_size defaults to 1 when it is unspecified for dynamic or guided schedule. The chunk size cannot be specified for runtime or auto schedule. But in implementation, llvm openmp runtime uses kmpc_dispatch_init with the argument "chunk_size" of 1 for runtime or auto schedule.

To be short, schedule(static) is NOT the same as schedule(static,1).
schedule(dynamic) is the same as schedule(dynamic,1).
schedule(guided) is the same as schedule(guided,1).
schedule(runtime, 1) and schedule(auto, 1) is not allowed, but schedule(runtime) and schedule(auto) behave like schedule(runtime, 1) and schedule(auto, 1) in IR level.

I noticed "if (!chunk) chunk = one;" is also added in `applyDynamicWorkshareLoop` in OMPIRBuilder, so remove this code here.


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

https://reviews.llvm.org/D114331