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

[Mats Petersson via Phabricator via llvm-commits]

Leporacanthicus added a comment.

Generally looks good to me. Just one nit-pick and one "I don't really understand how this actually produces the correct result" (may be that I'm just missing something!)



================
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);
----------------
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? 


================
Comment at: mlir/test/Target/LLVMIR/openmp-llvm.mlir:457
+
+llvm.func @test_omp_wsloop_static_2(%lb : i32, %ub : i32, %step : i32) -> () {
+ %const_i32 = llvm.mlir.constant(2 : i32) : i32
----------------
I'd like to see this having a separate name - it makes it so much easier to search the source code when the tests have distinct names! :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114331