This is an archive of the discontinued LLVM Phabricator instance.

Differential D59871

[AMDGPU] Avoid mistaking a ds atomic for a load
AbandonedPublic

Authored by tpr on Mar 27 2019, 2:13 AM.

Details

Reviewers
arsenm
Summary

SIInstrInfo::areLoadsFromSameBasePtr was mistaking a ds atomic for a
load, and then asserting when there was a non-constant operand where it
was expecting to find the load constant offset.

Change-Id: I484b3e240d0a25f34846a8cafbaf2befb37f8c07

Diff Detail

Event Timeline

tpr created this revision.Mar 27 2019, 2:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 27 2019, 2:13 AM
Herald added subscribers: llvm-commits, jfb, t-tye and 7 others. · View Herald Transcript
Harbormaster completed remote builds in B29680: Diff 192418.Mar 27 2019, 2:14 AM
tpr added a reviewer: arsenm.Mar 27 2019, 2:14 AM
arsenm added a comment.Mar 27 2019, 7:19 AM

Duplicate D59851, except mine still allows clustering

tpr abandoned this revision.Mar 27 2019, 9:46 AM

OK thanks Matt.

Revision Contents

PathSize
lib/
Target/
AMDGPU/
5 lines
test/
CodeGen/
AMDGPU/
27 lines

Diff 192418

lib/Target/AMDGPU/SIInstrInfo.cpp

Show First 20 Lines • Show All 144 Lines • ▼ Show 20 Linesbool SIInstrInfo::areLoadsFromSameBasePtr(SDNode *Load0, SDNode *Load1,
int64_t &Offset0, int64_t &Offset0,
int64_t &Offset1) const { int64_t &Offset1) const {
if (!Load0->isMachineOpcode() || !Load1->isMachineOpcode()) if (!Load0->isMachineOpcode() || !Load1->isMachineOpcode())
return false; return false;
unsigned Opc0 = Load0->getMachineOpcode(); unsigned Opc0 = Load0->getMachineOpcode();
unsigned Opc1 = Load1->getMachineOpcode(); unsigned Opc1 = Load1->getMachineOpcode();
// Make sure both are actually loads. // Make sure both are actually loads (not stores or atomics).
if (!get(Opc0).mayLoad() || !get(Opc1).mayLoad()) if (!get(Opc0).mayLoad() || get(Opc0).mayStore() || !get(Opc1).mayLoad() ||
get(Opc1).mayStore())
return false; return false;
if (isDS(Opc0) && isDS(Opc1)) { if (isDS(Opc0) && isDS(Opc1)) {
// FIXME: Handle this case: // FIXME: Handle this case:
if (getNumOperandsNoGlue(Load0) != getNumOperandsNoGlue(Load1)) if (getNumOperandsNoGlue(Load0) != getNumOperandsNoGlue(Load1))
return false; return false;
▲ Show 20 Lines • Show All 5,538 Lines • Show Last 20 Lines

test/CodeGen/AMDGPU/two-ds-atomics.ll

  • This file was added.
; RUN: llc -march=amdgcn -mcpu=gfx900 -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
; This test case asserted due to SIInstrInfo::areLoadsFromSameBasePtr mistaking
; an atomic for a load.
; GCN-LABEL: _amdgpu_cs_main:
; GCN: ds_max_u32
; GCN: ds_min_u32
@TileFarZ = external dso_local addrspace(3) global i32, align 16
@TileNearZ = external dso_local addrspace(3) global i32, align 16
define amdgpu_cs void @_amdgpu_cs_main() local_unnamed_addr #0 {
.entry:
%tmp = tail call float @llvm.amdgcn.image.load.mip.2d.f32.i32(i32 1, i32 undef, i32 undef, i32 0, <8 x i32> undef, i32 0, i32 0)
%tmp1 = bitcast float %tmp to i32
%tmp2 = atomicrmw volatile umax i32 addrspace(3)* @TileFarZ, i32 %tmp1 seq_cst
%tmp3 = atomicrmw volatile umin i32 addrspace(3)* @TileNearZ, i32 %tmp1 seq_cst
unreachable
}
; Function Attrs: nounwind readonly
declare float @llvm.amdgcn.image.load.mip.2d.f32.i32(i32 immarg, i32, i32, i32, <8 x i32>, i32 immarg, i32 immarg) #1
attributes #0 = { "target-features" }
attributes #1 = { nounwind readonly }