[PATCH] D92439: [CLANG] Fix missing error for use of 128-bit integer inside SPIR64 device code.

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 added inline comments.


================
Comment at: clang/lib/Sema/Sema.cpp:241
+      (Context.getAuxTargetInfo() &&
+       Context.getAuxTargetInfo()->hasInt128Type())) {
     // If either of the 128-bit integer types are unavailable to name lookup,
----------------
jdoerfert wrote:
> jyu2 wrote:
> > jyu2 wrote:
> > > jdoerfert wrote:
> > > > jyu2 wrote:
> > > > > jdoerfert wrote:
> > > > > > I don't understand why this (and the changes below) are necessary. Host and device compilation are separate. This should not be any different to CUDA, HIP, or OpenMP offload which seem not to require this.
> > > > > As far as I can know, in real compile, the auxtarget should be also set for example SYCL.  May be that is only for our compiler.  I am not sure what do you mean by it should same with CUDA, HIP...?  Do you mean, CUDA(or HIP...) target should also not support 128-bit integer?  If so, I don't see any error emit for CUDA when I try with nvptx64-nvidia-cuda.
> > > > > 
> > > > > Thanks.
> > > > > 
> > > > I'm not saying auxtarget is not set. I'm trying to understand why this is necessary. Why would you initialize `__int128_t` if your target doesn't support it? What happens if you only include the SPIR.h change?
> > > Without this change you will see error:
> > > 
> > > j3.c:2:15: error: unknown type name '__uint128_t'
> > > typedef const __uint128_t  megeType;
> > > 
> > > bash-4.4$ cat j3.c
> > > 
> > > typedef const __uint128_t  megeType;
> > > 
> > > Without change in SemaOverload.c:
> > > you will see error:
> > > x.cpp:3:14: error: use of overloaded operator '==' is ambiguous (with operand types 'struct X' and '__int128')
> > >   bool a = x == __int128(0);
> > >            ~ ^  ~~~~~~~~~~~
> > > 
> > > bash-4.4$ cat x.cpp
> > > namespace PR12964 {
> > >   struct X { operator  __int128() const; } x;
> > >   bool a = x == __int128(0);
> > > }
> > > 
> > > 
> > > 
> > Just one more point, the errors only need to be emitted for 128-bit integer used inside device code.  The test case above 128-bit integer is not used in device code, so we don't want to emit error for it.
> > 
> > Thanks.
> > Jennifer
> I see. You want the types to be there so the diagnostic says the types are not available. OK, fine by me.
Thank you so much for your time for review this!!!


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

https://reviews.llvm.org/D92439