bader added inline comments.
================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1635
+ Context.getTargetInfo().getConstantAddressSpace().value_or(
+ LangAS::Default));
llvm::Constant *GlobalConstStr = Builder.CreateGlobalStringPtr(
----------------
arichardson wrote:
> arichardson wrote:
> > bader wrote:
> > > > This changes the code generation for spir64 to place the globals in
> > > > addrspace(4). I believe is correct, but it would be good for someone
> > > > who is familiar with the target to confirm.
> > >
> > > Globals must reside in `sycl_global` namespace, which is `addrspace(1)`
> > > for spir* targets.
> > > `addrspace(4)` represents "generic" address space, which is a placeholder
> > > for a specific address space. If we leave it `addrspace(4)` for global
> > > definition, the compiler won't be able to infer genuine address space.
> > Okay that's interesting - I guess it means we should not be using
> > `getConstantAddressSpace()` here? Or getConstantAddressSpace() should
> > actually return a value that maps to `addrspace(1)`?
> Ah it looks like we should be using
> `CodeGenModule::GetGlobalConstantAddressSpace()` instead of
> `getTarget().getConstantAddressSpace()`. Is that correct?
>
>
> ```
> LangAS CodeGenModule::GetGlobalConstantAddressSpace() const {
> // OpenCL v1.2 s6.5.3: a string literal is in the constant address space.
> if (LangOpts.OpenCL)
> return LangAS::opencl_constant;
> if (LangOpts.SYCLIsDevice)
> return LangAS::sycl_global;
> if (LangOpts.HIP && LangOpts.CUDAIsDevice && getTriple().isSPIRV())
> // For HIPSPV map literals to cuda_device (maps to CrossWorkGroup in
> SPIR-V)
> // instead of default AS (maps to Generic in SPIR-V). Otherwise, we end up
> // with OpVariable instructions with Generic storage class which is not
> // allowed (SPIR-V V1.6 s3.42.8). Also, mapping literals to SPIR-V
> // UniformConstant storage class is not viable as pointers to it may not
> be
> // casted to Generic pointers which are used to model HIP's "flat"
> pointers.
> return LangAS::cuda_device;
> if (auto AS = getTarget().getConstantAddressSpace())
> return *AS;
> return LangAS::Default;
> }
> ```
>
> Another problem appears to be that the default implementation of
> getConstantAddressSpace() returns `LangAS::Default` instead of None, so the
> .value_or() will never be used.
> Ah it looks like we should be using
> CodeGenModule::GetGlobalConstantAddressSpace() instead of
> getTarget().getConstantAddressSpace(). Is that correct?
Yes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138284/new/
https://reviews.llvm.org/D138284
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
