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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to