diff options
| author | Tim Renouf <tpr.llvm@botech.co.uk> | 2018-05-25 07:55:04 +0000 |
|---|---|---|
| committer | Tim Renouf <tpr.llvm@botech.co.uk> | 2018-05-25 07:55:04 +0000 |
| commit | ad8b7c1190b963e9df1757fbb28f14f2f34ec9a2 (patch) | |
| tree | a89b97b0a729b8a699e572dbd66048afd72a5513 /llvm/lib | |
| parent | b161db099db7d611c14f64d36a57767f62d0a430 (diff) | |
| download | bcm5719-llvm-ad8b7c1190b963e9df1757fbb28f14f2f34ec9a2.tar.gz bcm5719-llvm-ad8b7c1190b963e9df1757fbb28f14f2f34ec9a2.zip | |
[AMDGPU] Fixed incorrect break from loop
Summary:
Lower control flow did not correctly handle the case that a loop break
in if/else was on a condition that was not guaranteed to be masked by
exec. The first test kernel shows an example of this going wrong; after
exiting the loop, exec is all ones, even if it was not before the loop.
The fix is for lowering of if-break and else-break to insert an
S_AND_B64 to mask the break condition with exec. This commit also
includes the optimization of not inserting that S_AND_B64 if it is
obviously not needed because the break condition is the result of a
V_CMP in the same basic block.
V2: Addressed some review comments.
V3: Test fixes.
Subscribers: arsenm, kzhuravl, wdng, nhaehnle, yaxunl, dstuttard, t-tye, llvm-commits
Differential Revision: https://reviews.llvm.org/D44046
Change-Id: I0fc56a01209a9e99d1d5c9b0ffd16f111caf200c
llvm-svn: 333258
Diffstat (limited to 'llvm/lib')
| -rw-r--r-- | llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp | 42 |
1 files changed, 40 insertions, 2 deletions
diff --git a/llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp b/llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp index 72d3c49f76e..a8426c3039f 100644 --- a/llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp +++ b/llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp @@ -344,11 +344,49 @@ void SILowerControlFlow::emitBreak(MachineInstr &MI) { } void SILowerControlFlow::emitIfBreak(MachineInstr &MI) { - MI.setDesc(TII->get(AMDGPU::S_OR_B64)); + MachineBasicBlock &MBB = *MI.getParent(); + const DebugLoc &DL = MI.getDebugLoc(); + auto Dst = MI.getOperand(0).getReg(); + + // Skip ANDing with exec if the break condition is already masked by exec + // because it is a V_CMP in the same basic block. (We know the break + // condition operand was an i1 in IR, so if it is a VALU instruction it must + // be one with a carry-out.) + bool SkipAnding = false; + if (MI.getOperand(1).isReg()) { + if (MachineInstr *Def = MRI->getUniqueVRegDef(MI.getOperand(1).getReg())) { + SkipAnding = Def->getParent() == MI.getParent() + && SIInstrInfo::isVALU(*Def); + } + } + + // AND the break condition operand with exec, then OR that into the "loop + // exit" mask. + MachineInstr *And = nullptr, *Or = nullptr; + if (!SkipAnding) { + And = BuildMI(MBB, &MI, DL, TII->get(AMDGPU::S_AND_B64), Dst) + .addReg(AMDGPU::EXEC) + .add(MI.getOperand(1)); + Or = BuildMI(MBB, &MI, DL, TII->get(AMDGPU::S_OR_B64), Dst) + .addReg(Dst) + .add(MI.getOperand(2)); + } else + Or = BuildMI(MBB, &MI, DL, TII->get(AMDGPU::S_OR_B64), Dst) + .add(MI.getOperand(1)) + .add(MI.getOperand(2)); + + if (LIS) { + if (And) + LIS->InsertMachineInstrInMaps(*And); + LIS->ReplaceMachineInstrInMaps(MI, *Or); + } + + MI.eraseFromParent(); } void SILowerControlFlow::emitElseBreak(MachineInstr &MI) { - MI.setDesc(TII->get(AMDGPU::S_OR_B64)); + // Lowered in the same way as emitIfBreak above. + emitIfBreak(MI); } void SILowerControlFlow::emitLoop(MachineInstr &MI) { |

