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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
