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

[Mats Petersson via Phabricator via llvm-commits]

Leporacanthicus 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);
----------------
peixin wrote:
> 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.
Thanks, I obviously got confused by the "hidden" bit of code, and thought it was just continuation of the static wsloop code.


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

https://reviews.llvm.org/D114331