From d11b93ec6ac1cf48dce0a8b7beb3e07f0ee9b0fc Mon Sep 17 00:00:00 2001 From: Austin Kerbow Date: Mon, 28 Oct 2019 09:39:20 -0700 Subject: [PATCH] AMDGPU: Avoid overwriting saved PC Summary: An outstanding load with same destination sgpr as call could cause PC to be updated with junk value on return. Reviewers: arsenm, rampitec Reviewed By: arsenm Subscribers: kzhuravl, jvesely, wdng, nhaehnle, yaxunl, dstuttard, tpr, t-tye, hiraditya, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D69474 --- llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp | 26 ++++++++++--- llvm/test/CodeGen/AMDGPU/call-waw-waitcnt.mir | 53 +++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 6 deletions(-) create mode 100644 llvm/test/CodeGen/AMDGPU/call-waw-waitcnt.mir diff --git a/llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp b/llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp index dcb04e4..e84e948 100644 --- a/llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp +++ b/llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp @@ -939,19 +939,33 @@ bool SIInsertWaitcnts::generateWaitcntInstBefore( } if (MI.isCall() && callWaitsOnFunctionEntry(MI)) { - // Don't bother waiting on anything except the call address. The function - // is going to insert a wait on everything in its prolog. This still needs - // to be careful if the call target is a load (e.g. a GOT load). + // The function is going to insert a wait on everything in its prolog. + // This still needs to be careful if the call target is a load (e.g. a GOT + // load). We also need to check WAW depenancy with saved PC. Wait = AMDGPU::Waitcnt(); int CallAddrOpIdx = AMDGPU::getNamedOperandIdx(MI.getOpcode(), AMDGPU::OpName::src0); - RegInterval Interval = ScoreBrackets.getRegInterval(&MI, TII, MRI, TRI, - CallAddrOpIdx, false); - for (signed RegNo = Interval.first; RegNo < Interval.second; ++RegNo) { + RegInterval CallAddrOpInterval = ScoreBrackets.getRegInterval( + &MI, TII, MRI, TRI, CallAddrOpIdx, false); + + for (signed RegNo = CallAddrOpInterval.first; + RegNo < CallAddrOpInterval.second; ++RegNo) ScoreBrackets.determineWait( LGKM_CNT, ScoreBrackets.getRegScore(RegNo, LGKM_CNT), Wait); + + int RtnAddrOpIdx = + AMDGPU::getNamedOperandIdx(MI.getOpcode(), AMDGPU::OpName::dst); + if (RtnAddrOpIdx != -1) { + RegInterval RtnAddrOpInterval = ScoreBrackets.getRegInterval( + &MI, TII, MRI, TRI, RtnAddrOpIdx, false); + + for (signed RegNo = RtnAddrOpInterval.first; + RegNo < RtnAddrOpInterval.second; ++RegNo) + ScoreBrackets.determineWait( + LGKM_CNT, ScoreBrackets.getRegScore(RegNo, LGKM_CNT), Wait); } + } else { // FIXME: Should not be relying on memoperands. // Look at the source operands of every instruction to see if diff --git a/llvm/test/CodeGen/AMDGPU/call-waw-waitcnt.mir b/llvm/test/CodeGen/AMDGPU/call-waw-waitcnt.mir new file mode 100644 index 0000000..e3021cf --- /dev/null +++ b/llvm/test/CodeGen/AMDGPU/call-waw-waitcnt.mir @@ -0,0 +1,53 @@ +# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py +# RUN: llc -march=amdgcn -mcpu=gfx900 -verify-machineinstrs -run-pass=si-insert-waitcnts %s -o - | FileCheck -check-prefix=GCN %s + +# $sgpr30_sgpr31 will hold the return address. We need a waitcnt before SI_CALL so +# that the return address is not clobbered in the callee by the outstanding load. + +--- | + define amdgpu_kernel void @call_waw_waitcnt() #0 { + %1 = call i32 @func() + ret void + } + + declare hidden i32 @func() #0 + + attributes #0 = { nounwind } +... + +--- +name: call_waw_waitcnt +tracksRegLiveness: true +body: | + bb.0: + liveins: $sgpr4_sgpr5, $sgpr7, $sgpr0_sgpr1_sgpr2_sgpr3 + + ; GCN-LABEL: name: call_waw_waitcnt + ; GCN: liveins: $sgpr4_sgpr5, $sgpr7, $sgpr0_sgpr1_sgpr2_sgpr3 + ; GCN: S_WAITCNT 0 + ; GCN: $sgpr30_sgpr31 = S_LOAD_DWORDX2_IMM $sgpr4_sgpr5, 0, 0, 0 + ; GCN: $sgpr33 = S_MOV_B32 killed $sgpr7 + ; GCN: $flat_scr_lo = S_ADD_U32 killed $sgpr4, $sgpr33, implicit-def $scc + ; GCN: $flat_scr_hi = S_ADDC_U32 killed $sgpr5, 0, implicit-def $scc, implicit killed $scc + ; GCN: BUNDLE implicit-def $sgpr4_sgpr5, implicit-def $sgpr4, implicit-def $sgpr5, implicit-def $scc { + ; GCN: $sgpr4_sgpr5 = S_GETPC_B64 + ; GCN: $sgpr4 = S_ADD_U32 internal $sgpr4, target-flags(amdgpu-rel32-lo) @func + 4, implicit-def $scc + ; GCN: $sgpr5 = S_ADDC_U32 internal $sgpr5, target-flags(amdgpu-rel32-hi) @func + 4, implicit-def $scc, implicit internal $scc + ; GCN: } + ; GCN: $sgpr32 = S_MOV_B32 $sgpr33 + ; GCN: S_WAITCNT 49279 + ; GCN: dead $sgpr30_sgpr31 = SI_CALL killed renamable $sgpr4_sgpr5, @func, csr_amdgpu_highregs, implicit $sgpr0_sgpr1_sgpr2_sgpr3, implicit-def dead $vgpr0 + ; GCN: S_ENDPGM 0 + $sgpr30_sgpr31 = S_LOAD_DWORDX2_IMM $sgpr4_sgpr5, 0, 0, 0 + $sgpr33 = S_MOV_B32 killed $sgpr7 + $flat_scr_lo = S_ADD_U32 killed $sgpr4, $sgpr33, implicit-def $scc + $flat_scr_hi = S_ADDC_U32 killed $sgpr5, 0, implicit-def $scc, implicit killed $scc + BUNDLE implicit-def $sgpr4_sgpr5, implicit-def $sgpr4, implicit-def $sgpr5, implicit-def $scc { + $sgpr4_sgpr5 = S_GETPC_B64 + $sgpr4 = S_ADD_U32 internal $sgpr4, target-flags(amdgpu-rel32-lo) @func + 4, implicit-def $scc + $sgpr5 = S_ADDC_U32 internal $sgpr5, target-flags(amdgpu-rel32-hi) @func + 4, implicit-def $scc, implicit internal $scc + } + $sgpr32 = S_MOV_B32 $sgpr33 + dead $sgpr30_sgpr31 = SI_CALL killed renamable $sgpr4_sgpr5, @func, csr_amdgpu_highregs, implicit $sgpr0_sgpr1_sgpr2_sgpr3, implicit-def dead $vgpr0 + S_ENDPGM 0 +... -- 2.7.4