abidmalikwaterloo marked 2 inline comments as done. abidmalikwaterloo added inline comments.
================ Comment at: clang/lib/Testing/CMakeLists.txt:6 +======= +>>>>>>> [Testing] TestAST, a helper for writing straight-line AST tests # Not add_clang_library: this is not part of clang's public library interface. ---------------- shraiysh wrote: > nit: Merge conflict. Please remove. removed ================ Comment at: clang/lib/Testing/CMakeLists.txt:33 + clangBasic + clangFrontend ) ---------------- shraiysh wrote: > abidmalikwaterloo wrote: > > Are these okay? In D105584 these were not > No, this is not okay. There is some issue with these patches and I don't know > what it is, but these unrelated changes should not be seen here. Thank you > for pointing this out :) removed them manually ================ Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:594 + + The $map_operands specifies operands with no map type + ---------------- shraiysh wrote: > Data mapping is very similar to data privatization. Would it be a good idea > to push this to the frontends while we have not concretely decided on openmp > dialect privatization/mapping? If you have an idea on how to implement > mapping in OpenMP Dialect to LLVM IR maybe we can go ahead with this. > > CC @kiranchandramohan for suggestions regarding privatization. @kiranchandramohan @raghavendhra I would like to keep it like this! Any comments ================ Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:661 + + The $map_type_modifier vector specifies the modifier for each map type + operand ---------------- shraiysh wrote: > It is not clear which element in this list corresponds to which map type. It > would be nice if we could either document that here, or have separate > operands for each map type. (same for other two constructs too) I feel keeping it in vector makes more sense. I will document it here. As per OpenMP Specifications: The map-type can be **to, from, tofrom, or alloc**. ================ Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:718 + operand + + TODO: depend ---------------- will document map type here as wekk Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105255/new/ https://reviews.llvm.org/D105255 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
