[PATCH] D89980: [hip] Remove kernel argument coercion.

Wed Oct 28 10:22:18 PDT 2020

arsenm added a comment.

In D89980#2357208 <https://reviews.llvm.org/D89980#2357208>, @hliao wrote:

> Besides the unpromotable `alloca` issue due to indirect accesses, such coercion to GLOBAL pointer directly is not safe as, in HIP/CUDA, both CONSTANT and GLOBAL pointers would be passed as the kernel arguments. Without introducing a new address space combing GLOBAL/CONSTANT, such coercion would be unsafe.

I'm not sure what you are saying here. Constant address space doesn't really exist and we could eliminate it. It is always valid to use global in place of constant.

================
Comment at: clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu:19
+// COMMON-LABEL: define amdgpu_kernel void @_Z7kernel1Pi(i32*{{.*}} %x)
+// OPT: [[VAL:%.*]] = load i32, i32* %x, align 4
 // OPT: [[INC:%.*]] = add nsw i32 [[VAL]], 1
----------------
hliao wrote:
> arsenm wrote:
> > hliao wrote:
> > > arsenm wrote:
> > > > hliao wrote:
> > > > > arsenm wrote:
> > > > > > This is still a regression. Fixing up AA does not solve the problem this promotions this is intended to solve. Generic accesses are worse independently of the aliasing properties
> > > > > Do you mean FLAT load/store has worse addressing mode than GLOBAL ones?
> > > > Yes. The flat offsets have a smaller range, and do not have the saddr mode. Flat accesses also won't avoid the extra lgmkcnt wait
> > > I plan to add support to select GLOBAL ones once we could confirm that pointer could only point to GLOBAL/CONSTANT address spaces. Do you think that's a reasonable solution?
> > I would much rather have the IR express the address space rather than fixing it up later. IR passes are aware of the addressing mode differences. Relying on AA for basic selection would also be worse for compile time
> LLVM IR is agnostic to the underlying addressing mode. In code selection, we won't use AA but just needs to check IR.
It's not really. The codegen IR passes check the addressing mode based on the indexed address space. Using a flat pointer is strictly worse than an addrspace(1) pointer

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89980