[flang-commits] [llvm] [flang] [flang][OpenMP] Add semantic check for declare target (PR #71861)

via flang-commits flang-commits at lists.llvm.org
Fri Nov 10 08:16:11 PST 2023


agozillon wrote:

LGTM, but likely a good idea to wait on one more reviewer. 

However, I would perhaps change the name and commit message of the PR to indicate that this patch also adds the Enter clause to declare target! 

I'll also point out that it likely won't work in the lowering the same way as To currently does (perhaps there's another PR for this that I've managed to miss, sorry if that's the case), as there's no handling of it, so OpenMP.cpp needs to be taught how to handle it and lower it to MLIR. Which might be a problem if we're indicating to use Enter rather than To, which has some support currently I believe (not as thoroughly tested as link though). 

You could probably just teach it that enter and to are the same, and treat both cases as a To, as they're synonyms. However, you can also feel free to add Enter (and probably indirect, even if there's no handling for it yet) to the MLIR omp dialects enum and then alter the MLIR -> LLVM-IR lowering to incorporate the idea of enter (would likely just be change enter -> to or just add an enter OR to for everywhere to shows up currently). Not the biggest deal but might be a bit of an issue if we're actively warning users to use the other that has less support.  

https://github.com/llvm/llvm-project/pull/71861


More information about the flang-commits mailing list