[PATCH] R600: Remove predicated_break inst
Tom Stellard
tom at stellard.net
Fri Jul 26 21:05:27 PDT 2013
On Fri, Jul 26, 2013 at 02:45:36PM +0200, Vincent Lejeune wrote:
> We were using two instructions for similar purpose : break and
> predicated break. Only predicated_break was emitted and it was
> lowered at R600ControlFlowFinalizer to JUMP;CF_BREAK;POP.
> This commit simplify the situation by making AMDILCFGStructurizer
> emit IF_PREDICATE;BREAK;ENDIF; instead of predicated_break (which
> is now removed).
>
> There is no functionality change.
This patch is Reviewed-by: Tom Stellard <thomas.stellard at amd.com>
A few minor comments below.
> ---
> lib/Target/R600/AMDILCFGStructurizer.cpp | 21 ++++-----------------
> lib/Target/R600/R600ControlFlowFinalizer.cpp | 11 ++---------
> lib/Target/R600/R600ExpandSpecialInstrs.cpp | 15 ---------------
> lib/Target/R600/R600Instructions.td | 3 ---
> 4 files changed, 6 insertions(+), 44 deletions(-)
>
> diff --git a/lib/Target/R600/AMDILCFGStructurizer.cpp b/lib/Target/R600/AMDILCFGStructurizer.cpp
> index fac56f0..e1687b0 100644
> --- a/lib/Target/R600/AMDILCFGStructurizer.cpp
> +++ b/lib/Target/R600/AMDILCFGStructurizer.cpp
> @@ -669,12 +669,6 @@ MachineInstr *AMDGPUCFGStructurizer::getContinueInstr(MachineBasicBlock *MBB) {
> }
>
> MachineInstr *AMDGPUCFGStructurizer::getLoopBreakInstr(MachineBasicBlock *MBB) {
> - for (MachineBasicBlock::iterator It = MBB->begin(); (It != MBB->end());
> - ++It) {
> - MachineInstr *MI = &(*It);
> - if (MI->getOpcode() == AMDGPU::PREDICATED_BREAK)
> - return MI;
> - }
> return NULL;
> }
Do we still need this function? All it does now is return NULL.
>
> @@ -1529,16 +1523,7 @@ void AMDGPUCFGStructurizer::mergeLooplandBlock(MachineBasicBlock *DstBlk,
> DEBUG(dbgs() << "loopPattern header = BB" << DstBlk->getNumber()
> << " land = BB" << LandMBB->getNumber() << "\n";);
>
> - /* we last inserterd the DebugLoc in the
> - * BREAK_LOGICALZ_i32 or AMDGPU::BREAK_LOGICALNZ statement in the current
> - * dstBlk.
> - * search for the DebugLoc in the that statement.
> - * if not found, we have to insert the empty/default DebugLoc */
> - MachineInstr *LoopBreakInstr = getLoopBreakInstr(DstBlk);
> - DebugLoc DLBreak = (LoopBreakInstr) ? LoopBreakInstr->getDebugLoc() :
> - DebugLoc();
> -
> - insertInstrBefore(DstBlk, AMDGPU::WHILELOOP, DLBreak);
> + insertInstrBefore(DstBlk, AMDGPU::WHILELOOP, DebugLoc());
>
> /* we last inserterd the DebugLoc in the continue statement in the current
> * dstBlk.
> @@ -1565,7 +1550,9 @@ void AMDGPUCFGStructurizer::mergeLoopbreakBlock(MachineBasicBlock *ExitingMBB,
> MachineBasicBlock::iterator I = BranchMI;
> if (TrueBranch != LandMBB)
> reversePredicateSetter(I);
> - insertCondBranchBefore(I, AMDGPU::PREDICATED_BREAK, DL);
> + insertCondBranchBefore(ExitingMBB, I, AMDGPU::IF_PREDICATE_SET, AMDGPU::PREDICATE_BIT, DL);
> + insertInstrBefore(I, AMDGPU::BREAK);
> + insertInstrBefore(I, AMDGPU::ENDIF);
> //now branchInst can be erase safely
> BranchMI->eraseFromParent();
> //now take care of successors, retire blocks
> diff --git a/lib/Target/R600/R600ControlFlowFinalizer.cpp b/lib/Target/R600/R600ControlFlowFinalizer.cpp
> index b69d38b..cc45891 100644
> --- a/lib/Target/R600/R600ControlFlowFinalizer.cpp
> +++ b/lib/Target/R600/R600ControlFlowFinalizer.cpp
> @@ -457,18 +457,11 @@ public:
> MI->eraseFromParent();
> break;
> }
> - case AMDGPU::PREDICATED_BREAK: {
> - CurrentStack--;
> - CfCount += 3;
> - BuildMI(MBB, MI, MBB.findDebugLoc(MI), getHWInstrDesc(CF_JUMP))
> - .addImm(CfCount)
> - .addImm(1);
> + case AMDGPU::BREAK: {
> + CfCount ++;
> MachineInstr *MIb = BuildMI(MBB, MI, MBB.findDebugLoc(MI),
> getHWInstrDesc(CF_LOOP_BREAK))
> .addImm(0);
> - BuildMI(MBB, MI, MBB.findDebugLoc(MI), getHWInstrDesc(CF_POP))
> - .addImm(CfCount)
> - .addImm(1);
> LoopStack.back().second.insert(MIb);
> MI->eraseFromParent();
> break;
> diff --git a/lib/Target/R600/R600ExpandSpecialInstrs.cpp b/lib/Target/R600/R600ExpandSpecialInstrs.cpp
> index efc9523..67b42d7 100644
> --- a/lib/Target/R600/R600ExpandSpecialInstrs.cpp
> +++ b/lib/Target/R600/R600ExpandSpecialInstrs.cpp
> @@ -89,21 +89,6 @@ bool R600ExpandSpecialInstrsPass::runOnMachineFunction(MachineFunction &MF) {
> MI.eraseFromParent();
> continue;
> }
> - case AMDGPU::BREAK: {
> - MachineInstr *PredSet = TII->buildDefaultInstruction(MBB, I,
> - AMDGPU::PRED_SETE_INT,
> - AMDGPU::PREDICATE_BIT,
> - AMDGPU::ZERO,
> - AMDGPU::ZERO);
> - TII->addFlag(PredSet, 0, MO_FLAG_MASK);
> - TII->setImmOperand(PredSet, AMDGPU::OpName::update_exec_mask, 1);
> -
> - BuildMI(MBB, I, MBB.findDebugLoc(I),
> - TII->get(AMDGPU::PREDICATED_BREAK))
> - .addReg(AMDGPU::PREDICATE_BIT);
> - MI.eraseFromParent();
> - continue;
> - }
>
> case AMDGPU::INTERP_PAIR_XY: {
> MachineInstr *BMI;
> diff --git a/lib/Target/R600/R600Instructions.td b/lib/Target/R600/R600Instructions.td
> index 9ff3897..178e081 100644
> --- a/lib/Target/R600/R600Instructions.td
> +++ b/lib/Target/R600/R600Instructions.td
> @@ -1883,9 +1883,6 @@ def VTX_READ_GLOBAL_128_cm : VTX_READ_128_cm <1,
> def IF_PREDICATE_SET : ILFormat<(outs), (ins GPRI32:$src),
> "IF_PREDICATE_SET $src", []>;
>
As a future TODO, it would be great if we could get rid of all the
instructions that use the old AMDIL registers like GPRI32. I think
there are only a few of them left. With these gone we could finally get rid of
the AMDGPUConvertToISA pass.
> -def PREDICATED_BREAK : ILFormat<(outs), (ins GPRI32:$src),
> - "PREDICATED_BREAK $src", []>;
> -
> //===----------------------------------------------------------------------===//
> // Pseudo instructions
> //===----------------------------------------------------------------------===//
> --
> 1.8.3.1
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list