This is an archive of the discontinued LLVM Phabricator instance.

Differential D39685

AMDGPU: Handle or in multi-use shl ptr combi
ClosedPublic

Authored by arsenm on Nov 6 2017, 7:23 AM.

Details

Reviewers
rampitec

Diff Detail

Event Timeline

arsenm created this revision.Nov 6 2017, 7:23 AM
Herald added subscribers: t-tye, tpr, dstuttard and 4 others. · View Herald TranscriptNov 6 2017, 7:23 AM
rampitec added inline comments.Nov 6 2017, 10:28 AM
lib/Target/AMDGPU/SIISelLowering.cpp
5218–5219
N0->hasOneUse() is more readable in my view.
arsenm added inline comments.Nov 13 2017, 11:12 PM
lib/Target/AMDGPU/SIISelLowering.cpp
5218–5219

I'm not sure what you mean, that would remove the early return

rampitec added inline comments.Nov 14 2017, 9:37 AM
lib/Target/AMDGPU/SIISelLowering.cpp
5218–5219

Hm... This changes original logic and contradicts the comment above. Before we did early return if there is only one use, just as written in the comment. Now you only do the transformation if there is one use.

arsenm updated this revision to Diff 122921.Nov 14 2017, 2:55 PM

Fix condition

rampitec accepted this revision.Nov 14 2017, 2:59 PM

LGTM

This revision is now accepted and ready to land.Nov 14 2017, 2:59 PM
arsenm closed this revision.Nov 14 2017, 3:46 PM

r318223

Revision Contents

PathSize
lib/
Target/
AMDGPU/
4 lines
test/
CodeGen/
AMDGPU/
34 lines

Diff 122921

lib/Target/AMDGPU/SIISelLowering.cpp

Show First 20 Lines • Show All 5,209 Lines • ▼ Show 20 LinesSDValue SITargetLowering::performSHLPtrCombine(SDNode *N,
unsigned AddrSpace, unsigned AddrSpace,
EVT MemVT, EVT MemVT,
DAGCombinerInfo &DCI) const { DAGCombinerInfo &DCI) const {
SDValue N0 = N->getOperand(0); SDValue N0 = N->getOperand(0);
SDValue N1 = N->getOperand(1); SDValue N1 = N->getOperand(1);
// We only do this to handle cases where it's profitable when there are // We only do this to handle cases where it's profitable when there are
// multiple uses of the add, so defer to the standard combine. // multiple uses of the add, so defer to the standard combine.
// TODO: Support or if ((N0.getOpcode() != ISD::ADD && N0.getOpcode() != ISD::OR) ||
if (N0.getOpcode() != ISD::ADD || N0->hasOneUse()) N0->hasOneUse())
rampitecUnsubmitted
Not Done
ReplyInline Actions
N0->hasOneUse() is more readable in my view.
rampitec: || N0->hasOneUse() is more readable in my view.
arsenmAuthorUnsubmitted
Not Done
ReplyInline Actions

I'm not sure what you mean, that would remove the early return

arsenm: I'm not sure what you mean, that would remove the early return
rampitecUnsubmitted
Not Done
ReplyInline Actions

Hm... This changes original logic and contradicts the comment above. Before we did early return if there is only one use, just as written in the comment. Now you only do the transformation if there is one use.

rampitec: Hm... This changes original logic and contradicts the comment above. Before we did early return…
return SDValue(); return SDValue();
const ConstantSDNode *CN1 = dyn_cast<ConstantSDNode>(N1); const ConstantSDNode *CN1 = dyn_cast<ConstantSDNode>(N1);
if (!CN1) if (!CN1)
return SDValue(); return SDValue();
const ConstantSDNode *CAdd = dyn_cast<ConstantSDNode>(N0.getOperand(1)); const ConstantSDNode *CAdd = dyn_cast<ConstantSDNode>(N0.getOperand(1));
if (!CAdd) if (!CAdd)
▲ Show 20 Lines • Show All 1,806 Lines • Show Last 20 Lines

test/CodeGen/AMDGPU/shl_add_ptr.ll

Show First 20 Lines • Show All 378 Lines • ▼ Show 20 Linesdefine void @shl_add_ptr_combine_2use_both_max_private_offset(i16 zeroext %idx.arg) #0 {
%shl1 = shl i32 %idx.add, 5 %shl1 = shl i32 %idx.add, 5
%ptr0 = inttoptr i32 %shl0 to i32* %ptr0 = inttoptr i32 %shl0 to i32*
%ptr1 = inttoptr i32 %shl1 to i32* %ptr1 = inttoptr i32 %shl1 to i32*
store volatile i32 9, i32* %ptr0 store volatile i32 9, i32* %ptr0
store volatile i32 10, i32* %ptr1 store volatile i32 10, i32* %ptr1
ret void ret void
} }
; GCN-LABEL: {{^}}shl_or_ptr_combine_2use_lds:
; GCN: v_lshlrev_b32_e32 [[SCALE0:v[0-9]+]], 3, v0
; GCN: ds_write_b32 [[SCALE0]], v{{[0-9]+}} offset:32
; GCN: v_lshlrev_b32_e32 [[SCALE1:v[0-9]+]], 4, v0
; GCN: ds_write_b32 [[SCALE1]], v{{[0-9]+}} offset:64
define void @shl_or_ptr_combine_2use_lds(i32 %idx) #0 {
%idx.add = or i32 %idx, 4
%shl0 = shl i32 %idx.add, 3
%shl1 = shl i32 %idx.add, 4
%ptr0 = inttoptr i32 %shl0 to i32 addrspace(3)*
%ptr1 = inttoptr i32 %shl1 to i32 addrspace(3)*
store volatile i32 9, i32 addrspace(3)* %ptr0
store volatile i32 10, i32 addrspace(3)* %ptr1
ret void
}
; GCN-LABEL: {{^}}shl_or_ptr_combine_2use_max_lds_offset:
; GCN-DAG: v_lshlrev_b32_e32 [[SCALE0:v[0-9]+]], 3, v0
; GCN-DAG: v_lshlrev_b32_e32 [[SCALE1:v[0-9]+]], 4, v0
; GCN-DAG: ds_write_b32 [[SCALE0]], v{{[0-9]+}} offset:65528
; GCN-DAG: v_or_b32_e32 [[ADD1:v[0-9]+]], 0x1fff0, [[SCALE1]]
; GCN: ds_write_b32 [[ADD1]], v{{[0-9]+$}}
define void @shl_or_ptr_combine_2use_max_lds_offset(i32 %idx) #0 {
%idx.add = or i32 %idx, 8191
%shl0 = shl i32 %idx.add, 3
%shl1 = shl i32 %idx.add, 4
%ptr0 = inttoptr i32 %shl0 to i32 addrspace(3)*
%ptr1 = inttoptr i32 %shl1 to i32 addrspace(3)*
store volatile i32 9, i32 addrspace(3)* %ptr0
store volatile i32 10, i32 addrspace(3)* %ptr1
ret void
}
attributes #0 = { nounwind } attributes #0 = { nounwind }
attributes #1 = { nounwind readnone } attributes #1 = { nounwind readnone }