This is an archive of the discontinued LLVM Phabricator instance.

Differential D51592

AMDGPU: Fix DAG divergence not reporting flat loads
ClosedPublic

Authored by arsenm on Sep 3 2018, 6:03 AM.

Details

Reviewers
scott.linder
Summary

Match behavior in DAG of r340343

Diff Detail

Event Timeline

arsenm created this revision.Sep 3 2018, 6:03 AM
Herald added subscribers: t-tye, tpr, dstuttard and 5 others. · View Herald TranscriptSep 3 2018, 6:03 AM
scott.linder added inline comments.Sep 4 2018, 8:24 AM
test/CodeGen/AMDGPU/dag-divergence.ll
6

What is the extra s here for, the register type? It doesn't seem to match the output without the patch. Maybe s_load{{.*}} s?

arsenm updated this revision to Diff 163875.Sep 4 2018, 11:09 AM

Fix test. It's supposed to skip s_load_dwordx

scott.linder accepted this revision.Sep 4 2018, 11:21 AM

LGTM, thanks!

This revision is now accepted and ready to land.Sep 4 2018, 11:21 AM
arsenm closed this revision.Sep 4 2018, 11:59 AM

r341393

Revision Contents

PathSize
lib/
Target/
AMDGPU/
8 lines
test/
CodeGen/
AMDGPU/
30 lines

Diff 163875

lib/Target/AMDGPU/SIISelLowering.cpp

  • This file is larger than 256 KB, so syntax highlighting is disabled by default.
Show First 20 Lines • Show All 9,200 Lines • ▼ Show 20 Lines case ISD::CopyFromReg:
else if (!AMDGPU::isEntryFunctionCC(FLI->Fn->getCallingConv())) else if (!AMDGPU::isEntryFunctionCC(FLI->Fn->getCallingConv()))
return true; return true;
} }
return !KDA || KDA->isDivergent(FLI->getValueFromVirtualReg(Reg)); return !KDA || KDA->isDivergent(FLI->getValueFromVirtualReg(Reg));
} }
} }
break; break;
case ISD::LOAD: { case ISD::LOAD: {
const LoadSDNode *L = dyn_cast<LoadSDNode>(N); const LoadSDNode *L = cast<LoadSDNode>(N);
// FIXME: Also needs to handle flat. unsigned AS = L->getAddressSpace();
if (L->getMemOperand()->getAddrSpace() == AMDGPUAS::PRIVATE_ADDRESS) // A flat load may access private memory.
return true; return AS == AMDGPUAS::PRIVATE_ADDRESS || AS == AMDGPUAS::FLAT_ADDRESS;
} break; } break;
case ISD::CALLSEQ_END: case ISD::CALLSEQ_END:
return true; return true;
break; break;
case ISD::INTRINSIC_WO_CHAIN: case ISD::INTRINSIC_WO_CHAIN:
{ {
} }
Show All 27 Lines

test/CodeGen/AMDGPU/dag-divergence.ll

  • This file was added.
; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=fiji -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
; GCN-LABEL: {{^}}private_load_maybe_divergent:
; GCN: buffer_load_dword
; GCN-NOT: s_load_dword s
; GCN: flat_load_dword
scott.linderUnsubmitted
Not Done
ReplyInline Actions

What is the extra s here for, the register type? It doesn't seem to match the output without the patch. Maybe s_load{{.*}} s?

scott.linder: What is the extra `s` here for, the register type? It doesn't seem to match the output without…
; GCN-NOT: s_load_dword s
define amdgpu_kernel void @private_load_maybe_divergent(i32 addrspace(4)* %k, i32* %flat) {
%load = load volatile i32, i32 addrspace(5)* undef, align 4
%gep = getelementptr inbounds i32, i32 addrspace(4)* %k, i32 %load
%maybe.not.uniform.load = load i32, i32 addrspace(4)* %gep, align 4
store i32 %maybe.not.uniform.load, i32 addrspace(1)* undef
ret void
}
; GCN-LABEL: {{^}}flat_load_maybe_divergent:
; GCN: s_load_dwordx4
; GCN-NOT: s_load
; GCN: flat_load_dword
; GCN-NOT: s_load
; GCN: flat_load_dword
; GCN-NOT: s_load
; GCN: flat_store_dword
define amdgpu_kernel void @flat_load_maybe_divergent(i32 addrspace(4)* %k, i32* %flat) {
%load = load i32, i32* %flat, align 4
%gep = getelementptr inbounds i32, i32 addrspace(4)* %k, i32 %load
%maybe.not.uniform.load = load i32, i32 addrspace(4)* %gep, align 4
store i32 %maybe.not.uniform.load, i32 addrspace(1)* undef
ret void
}