jdoerfert added a comment.
Still unsure if we should also error out for NVPTX but that is a different
story. Looks OK from my side, assuming you address the earlier comment.
Maybe someone else should accept though.
================
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,
----------------
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.
Repository:
rG LLVM Github Monorepo
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
