[flang-commits] [flang] [flang][openmp] Support importing module declare target globals (PR #213930)

[via flang-commits]

agozillon wrote:

> Thank you for looking at this. There are definitely some gaps.
> 

Thank you for the fixes and looking into those gaps, which I'm unfortunately aware there's a number! :-) 

> The enter clause could be automap, but automap has not been implemented for module files yet. Because of this I have just set automap to false for imported variables in this patch.
> 

That's more than reasonable!

> I also found that the variable list of a to clause is ignored by the module writer. I opened another pr with a 1 line change to fix this #213936
> 

Thank you for that fix, I am surprised it wasn't all of them that were ignored (and it's odd that only ::to was, would have expected ::enter as it came later).

> Another gap I noticed while working on this is how conflicting capture clauses applied to global variables are being handled (or not very handled I guess) in flang right now. A lowered declare target global can only have one capture clause on it, but before lowering it could even have all of them. How it gets lowered depends on the order of things.
> 
> If they are part of the same directive they get priority from highest to lowest in the order, enter, link, to, because this is the order the clauses are processed.
> 
> ```
> ! var gets capture clause enter
> !$omp declare target link(var) enter(var)
> ```
> 
> If there are multiple directives, its the first one that applies
> 
> ```
> ! var gets capture clause link
> !$omp declare target link(var)
> !$omp declare target enter(var)
> ```
> 
> , except if a subsequent directive has a different device_type, in which case it's the last one that counts
> 
> ```
> ! var gets capture clause to
> !$omp declare target link(var)
> !$omp declare target enter(var) device_type(nohost)
> !$omp declare target to(var) device_type(nohost)
> ```
> 
> The module writer writes all the clauses to the module file but not which one was actually used. In this patch I check the clauses in the ClauseSet on the symbol in the order, link, to, enter, and select the first one that matches. But if the symbol has more than one capture clause it is not guaranteed the same one is selected as when the exporting module itself was compiled.
> 
> But is there even a reason to allow conflicting capture clauses on the same variable at all? Or should there be a semantic check that rejects such programs?

I would double check the specification, it's been a fairly lengthy time since I have read that particular segment, however, if it specifies you cannot do it, then it is best suited as a semantic check that rejects programs with a reasonable user facing error message.

The semantic checks for a lot of things are a little lacking with regards to declare target (and some other areas), so you can quite easily create non-conformant code, but where possible we should try to fix these areas :-)

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