[PATCH] D123403: [OpenMP] Refactor OMPScheduleType enum.

[Peixin Qiao via Phabricator via llvm-commits]

peixin added a comment.

A few comments. Mostly nits.



================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:3767
+          /*HasMonotonicModifier=*/false, /*HasNonmonotonicModifier=*/false,
+          /*HasOrdedClause=*/false);
       return;
----------------



================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h:87
+  BaseAuto = 6,
+  BaseRuntime = 5,
+  BaseTrapezoidal = 7,
----------------
  BaseRuntime = 5,
  BaseAuto = 6,


================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h:135
+  UnorderedGuidedSimd = BaseGuidedSimd | ModifierUnordered,   // (46)
+  UnorderedRuntimeSimd = BaseRuntimeSimd | ModifierUnordered, // (47)
+
----------------
Why not using the following to be consistent with the name in kmp.h?
StaticBalancedChunked 
GuidedSimd
RuntimeSimd


================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:2114
+      DL, CLI, AllocaIP, /*NeedsBarrier=*/true, getSchedKind(SchedType),
+      ChunkVal, /*Simd*/ false,
+      (SchedType & omp::OMPScheduleType::ModifierMonotonic) ==
----------------
Nit


================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:2254
+      DL, CLI, AllocaIP, /*NeedsBarrier=*/true, OMP_SCHEDULE_Static, ChunkVal,
+      /*HasSimdModifier*/ false, /*HasMonotonicModifier*/ false,
+      /*HasNonmonotonicModifier*/ false,
----------------



================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:2255
+      /*HasSimdModifier*/ false, /*HasMonotonicModifier*/ false,
+      /*HasNonmonotonicModifier*/ false,
+      /*HasOrderedClause*/ true);
----------------



================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:2256
+      /*HasNonmonotonicModifier*/ false,
+      /*HasOrderedClause*/ true);
 
----------------



================
Comment at: mlir/test/Target/LLVMIR/openmp-llvm.mlir:809
  for (%iv) : i64 = (%lb) to (%ub) step (%step) {
-  // CHECK: call void @__kmpc_dispatch_init_8u(%struct.ident_t* @{{.*}}, i32 %{{.*}}, i32 1073741894, i64 1, i64 %{{.*}}, i64 1, i64 1)
+  // CHECK: call void @__kmpc_dispatch_init_8u(%struct.ident_t* @{{.*}}, i32 %{{.*}},                i32 70, i64 1, i64 %{{.*}},             i64 1, i64 1)
   // CHECK: call void @__kmpc_dispatch_fini_8u
----------------
Is this one tab?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123403