[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

Kiran Chandramohan via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 7 07:38:10 PST 2023


kiranchandramohan added a comment.

In D144864#4175332 <https://reviews.llvm.org/D144864#4175332>, @agozillon wrote:

> In D144864#4175257 <https://reviews.llvm.org/D144864#4175257>, @kiranchandramohan wrote:
>
>> LG. See one minor comment in the tests.
>>
>> I would prefer having an Interface for Target Modules if that could be made to work. I guess this can be taken up separately after https://reviews.llvm.org/D144883.
>
> Thank you Kiran, by interface for Target Modules do you mean a new omp.module or this type of external interface: https://mlir.llvm.org/docs/Interfaces/#external-models-for-attribute-operation-and-type-interfaces
>
> And quite possibly, @skatrak is currently looking into the review comments in the referenced patch (as he found similar use for the patch), so we shall see where the results lead to. Never opposed to improving on the infrastructure and this is something we will iterate on as we progress with offloading! :)

The latter one (external interface).



================
Comment at: flang/test/Lower/OpenMP/omp-is-device.f90:8-17
+!DEVICE: module attributes {{{.*}}, omp.is_device = true{{.*}}}
+!HOST: module attributes {{{.*}}, omp.is_device = false{{.*}}}
+!DEVICE-FLAG-ONLY: module attributes {{{.*}}"
+!DEVICE-FLAG-ONLY-NOT: , omp.is_device = {{.*}}
+!DEVICE-FLAG-ONLY-SAME: } 
+!BBC-DEVICE: module attributes {{{.*}}, omp.is_device = true{{.*}}}
+!BBC-HOST: module attributes {{{.*}}, omp.is_device = false{{.*}}}
----------------
agozillon wrote:
> kiranchandramohan wrote:
> > Any reason to have separate checks for essentially the same (e.g: DEVICE vs BBC-DEVICE)?
> In this case to test the addition of the flag to bbc and also to the Flang driver, and testing the flag has the same behavior in both. There is perhaps a way to make the test simpler that I'm unaware of though? e.g. a way to merge DEVICE/BBC-DEVICE into one check
If DEVICE and BBC-DEVICE are checking the same things, then you only need to have one of them (say DEVICE) and use it in the check-prefix of both.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144864



More information about the cfe-commits mailing list