[PATCH] D103655: [AMDGPU] Handle constant LDS uses from different kernels

Stanislav Mekhanoshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 7 12:45:41 PDT 2021


rampitec marked an inline comment as done.
rampitec added inline comments.


================
Comment at: llvm/test/CodeGen/AMDGPU/lower-module-lds-constantexpr.ll:43-45
+; CHECK: %4 = bitcast float addrspace(3)* getelementptr inbounds (%llvm.amdgcn.kernel.timestwo.lds.t, %llvm.amdgcn.kernel.timestwo.lds.t addrspace(3)* @llvm.amdgcn.kernel.timestwo.lds, i32 0, i32 0) to i32 addrspace(3)*
+; CHECK: %5 = addrspacecast i32 addrspace(3)* %4 to i32*
+; CHECK: %6 = ptrtoint i32* %5 to i64
----------------
rampitec wrote:
> rampitec wrote:
> > rampitec wrote:
> > > foad wrote:
> > > > Why do %4, %5, %6 need to be Instructions? Couldn't they could be left as ConstantExprs?
> > > It converts the whole constant expr, this is how D103661 works (and you have already commented there). I am not sure that is important though. Do you see any benefits of having long constant expressions vs instructions? We have to admit the testcase is quite degenerate too, more a torture test than a real life use.
> > Actually these have to be instructions. Reading this huge expression... It uses two lds globals, both constexprs are the innermost. If we replace an innermost expression with an instruction we then have to replace all constantexpr uses with instructions too since a constantexpr cannot use an instruction.
> Moreover, in our case we always replace an innermost expression as it all end up with a GlobalVariable. D103661 could probably stop replacing down the operands if it met the CE we requested to replace already, but that is never our scenario.
> 
> In this example we would always ask to replace @kern and @both users. I.e. an expression to pass to the helper is 'i32 addrspace(3)* bitcast (float addrspace(3)* @both to i32 addrspace(3)*)' for the first occurrence. It shall trigger the whole operand expression replacement.
> 
> However, helper function might have been improved to stop earlier if somebody would call it with a bigger expression, say 'i32* addrspacecast (i32 addrspace(3)* bitcast (float addrspace(3)* @both to i32 addrspace(3)*) to i32*'. If that would be a case convertConstantExprsToInstructions() might just stop with producing the addrspacecast instruction and do not convert the inner bitcast.
> Why do %4, %5, %6 need to be Instructions? Couldn't they could be left as ConstantExprs?

With the change in the D103661 we do not convert @both anymore, which is a believe a right thing, it stays in module lds. Only the tree to @kern is converted.


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

https://reviews.llvm.org/D103655



More information about the llvm-commits mailing list