[PATCH] D145591: [clang][HIP][OpenMP] Add warning if mixed HIP / OpenMP offloading

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8634
+def warn_hip_omp_target_directives : Warning<
+  "HIP does not support OpenMP target directives; directive has been ignored">,
+  InGroup<HIPOpenMPOffloading>;
----------------
yaxunl wrote:
> jdoerfert wrote:
> > yaxunl wrote:
> > > jdoerfert wrote:
> > > > mhalk wrote:
> > > > > jdoerfert wrote:
> > > > > > I doubt the ignored part very much.
> > > > > I just re-checked the LLVM IR of an example.
> > > > > With -x c++ there are __kmpc_target_(de)init calls; with -x hip there are none.
> > > > > https://godbolt.org/z/fhds46Pvc
> > > > > 
> > > > > From my point of view, "effectively", target directives seem to have been ignored.
> > > > > Maybe this is a coincidence (there's definitely some uncertainty) or I am misinterpreting the IR.
> > > > > I would be glad if you could shed some light on this, if possible / you find the time to do so.
> > > > Line 512 in the HIP output: https://godbolt.org/z/n5s5jz7j9.
> > > If I remove --offload-arch for the C++ program, I got the same result as for HIP:
> > > 
> > > https://godbolt.org/z/4TMzxdfj6
> > > 
> > > Does that mean we should emit the same error for C++ too?
> > I honestly do not follow your logic.
> > 
> > As I said in the beginning: I doubt the directive is ignored.
> > 
> > My link showed you that it doesn't ignore the pragma, e.g., the code w/ and w/o the pragma have different results.
> > This is also true for C++, but that doesn't make the warning any more accurate.
> > 
> I understand that the directive is not ignored and incorrect IR are emitted.
> 
> What I mean is that compiling "omp target" with HIP is equivalent to compiling C++ containing "omp target" without -fopenmp-targets specified in clang FE, therefore the error should be emitted in a more generic case. That is:
> 
> if "omp target" is compiled without -fopenmp-targets specified in FE, an error should be emitted.
> incorrect IR are emitted.

No, that IR is not incorrect. It is exactly what should be emitted.

> if "omp target" is compiled without -fopenmp-targets specified in FE, an error should be emitted.

No it should not. OpenMP target w/o an offloading target is well defined OpenMP.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145591