[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

Wed May 18 09:07:11 PDT 2022

abidmalikwaterloo added inline comments.

================
Comment at: clang/lib/Testing/CMakeLists.txt:29
   llvm_gtest
+  clangBasic
+  clangFrontend
----------------
shraiysh wrote:
> unrelated change?
When I rebase, these changes were highlighted in the main branch which was missing in the patch as it was too old.

================
Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:514
+    oilist(`schedule` `(`
+              custom<DistributeScheduleClause>(
+               $dist_schedule_var,
----------------
shraiysh wrote:
> dist_schedule is a dummy clause, where kind is only allowed to be `static` according to the standard. I don't think that requires a custom function, we can instead have something like the following -
> ```
> arguments = UnitAttr:$static_dist_schedule, Optional<IntLikeType>:$schedule_chunk
> 
> assemblyFormat = `dist_schedule` `(` (`static` $static_dist_schedule^)? (`:` $schedule_chunk^)? `)`
> ```
> Would that work? Let me know if there are any suggestions.
My only concern is; will this be a dummy clause with the static scheduler forever? I am pretty sure dist_schedule will have a dynamic or a  user defined scheduling strategy as well to improve the performance of a given application 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105584