[PATCH] D79937: [MLIR] Support for flush operation, and translating the same to LLVM IR

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan added inline comments.


================
Comment at: mlir/lib/Target/LLVMIR/ModuleTranslation.cpp:322
+  // flush with a list is treated same as a flush without a list by ignoring
+  // the list.
   return llvm::TypeSwitch<Operation *, LogicalResult>(&opInst)
----------------
Is this a TODO?
Do we require another version of CreateFlush in the OpenMP Builder to support the list?


================
Comment at: mlir/test/Target/openmp-llvm.mlir:4
+// CHECK-LABEL: define void @empty(i32 %0)
+llvm.func @empty(%arg0: !llvm.i32) {
   // CHECK: [[OMP_THREAD:%.*]] = call i32 @__kmpc_global_thread_num(%struct.ident_t* @{{[0-9]+}})
----------------
Shall we have more functions to test? Maybe one for those with arguments and one for those without? Also, rename this function.


================
Comment at: mlir/test/Target/openmp-llvm.mlir:20
+
+  // CHECK: call void @__kmpc_flush(%struct.ident_t* @{{[0-9]+}}
+  omp.flush %arg0, %arg0 : !llvm.i32, !llvm.i32
----------------
Is it possible to check that the arguments are also passed correctly for this version of omp.flush? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79937