Anastasia added inline comments.
================
Comment at: clang/include/clang/AST/Type.h:488
(A == LangAS::opencl_global && (B == LangAS::opencl_global_device ||
B == LangAS::opencl_global_host)) ||
// Consider pointer size address spaces to be equivalent to default.
----------------
bader wrote:
> BTW, we need enable `global_device` and `global_host` attributes from
> https://reviews.llvm.org/D82174 for SYCL USM feature. I have following
> question regarding this: should I create a follow-up patch or we can enable
> all attributes for SYCL at once?
It seems like they would just be extending the existing functionality and not
redesigning what we do in this patch?
If that's the case let's keep it in a separate patch, but feel free to upload
it even now.
================
Comment at: clang/lib/Basic/Targets/SPIR.h:129
+ TargetInfo::adjust(Opts);
+ setAddressSpaceMap(/*DefaultIsGeneric=*/Opts.SYCLIsDevice);
+ }
----------------
Anastasia wrote:
> Ok, do you still plan to add a `FIXME` as mentioned previously explaining why
> we need to reset the map here?
Btw I was just thinking of another alternative here.
What do you think about just setting a value in `Default` AS entry then we
wouldn't need any extra map at all in this case? So something like:
```
AddrSpaceMap[LangAS::Default] = AddrSpaceMap[LangAS::opencl_generic];
```
with a good explanation in `FIXME`? :)
================
Comment at: clang/test/CodeGenSYCL/convergent.cpp:2
// RUN: %clang_cc1 -fsycl-is-device -emit-llvm -disable-llvm-passes \
-// RUN: -triple spir64-unknown-unknown-sycldevice -emit-llvm %s -o - | \
// RUN: FileCheck %s
----------------
bader wrote:
> Anastasia wrote:
> > Is this change related? I thought we are not adding the environment
> > component after all...
> >
> >
> > Is this change related? I thought we are not adding the environment
> > component after all...
>
> While I was removing `-sycldevice` environment component from the patch, I
> noticed that one of the committed tests already uses it.
> https://github.com/llvm/llvm-project/blob/main/clang/test/CodeGenSYCL/convergent.cpp#L2
>
> Do you want to me to create a separate review request for this change?
I see, this looks trivial enough test clean up. You could just commit it
without any review perhaps?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89909/new/
https://reviews.llvm.org/D89909
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
