[PATCH] D119657: [OpenMP][mlir] Lowering for omp.atomic.update

[Peixin Qiao via Phabricator via llvm-commits]

peixin added inline comments.


================
Comment at: mlir/test/Target/LLVMIR/openmp-llvm.mlir:921
+  ^bb0(%xval: i32):
+    %t1 = llvm.mul %xval, %expr : i32
+    %t2 = llvm.sdiv %t1, %expr : i32
----------------
peixin wrote:
> I don't think you should process the lowering of the "expr" in MLIR stage. For different programing languages, the expr may have different structure. Can you try to analyze the "expr" when lowering parse-tree to MLIR? That is, the input for the "expr" in atomic update is always one mlir value and the LLVM IR generated for analyzing the "expr" is outside the atomic update region. Another reason for doing this is that "expr" cannot access the storage location designated by x, and the operations inside the atomic update region will be very clean. That is, you will always only have two instructions in atomic update region, one for update "xval", the other one for yield the "newval".
Similar to https://github.com/flang-compiler/f18-llvm-project/blob/9e96e8e4b1f84db0c30eaf4798f094f571b1c565/flang/lib/Lower/OpenMP.cpp#L320-L322. Using genExprValue should provide you the result value of the "expr".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119657