[Mesa-dev] [PATCH] radeon/llvm: rename if/break operator to improve readability

Tom Stellard tom at stellard.net
Wed Nov 28 11:10:44 PST 2012


On Wed, Nov 28, 2012 at 07:57:24PM +0100, Vincent Lejeune wrote:
> ---
>  lib/Target/AMDGPU/AMDILCFGStructurizer.cpp         | 36 +++++--------------
>  .../AMDGPU/MCTargetDesc/R600MCCodeEmitter.cpp      | 41 +++++-----------------
>  lib/Target/AMDGPU/R600ExpandSpecialInstrs.cpp      |  2 +-
>  lib/Target/AMDGPU/R600Instructions.td              | 11 ++++++
>  4 files changed, 29 insertions(+), 61 deletions(-)
> 

Nice cleanup, thanks!

Reviewed-by: Tom Stellard<thomas.stellard at amd.com>

> diff --git a/lib/Target/AMDGPU/AMDILCFGStructurizer.cpp b/lib/Target/AMDGPU/AMDILCFGStructurizer.cpp
> index 01a5d89..019feff 100644
> --- a/lib/Target/AMDGPU/AMDILCFGStructurizer.cpp
> +++ b/lib/Target/AMDGPU/AMDILCFGStructurizer.cpp
> @@ -1291,11 +1291,11 @@ int CFGStructurizer<PassT>::improveSimpleJumpintoIf(BlockT *headBlk,
>      CFGTraits::insertCompareInstrBefore(landBlk, insertPos, passRep, cmpResReg,
>                                          initReg, immReg);
>      CFGTraits::insertCondBranchBefore(landBlk, insertPos,
> -                                      AMDGPU::IF_LOGICALZ_i32, passRep,
> +                                      AMDGPU::IF_PREDICATE_SET, passRep,
>                                        cmpResReg, DebugLoc());
>    }
>  
> -  CFGTraits::insertCondBranchBefore(landBlk, insertPos, AMDGPU::IF_LOGICALNZ_i32,
> +  CFGTraits::insertCondBranchBefore(landBlk, insertPos, AMDGPU::IF_PREDICATE_SET,
>                                      passRep, initReg, DebugLoc());
>  
>    if (migrateTrue) {
> @@ -1546,7 +1546,7 @@ void CFGStructurizer<PassT>::mergeLooplandBlock(BlockT *dstBlk,
>    for (typename std::set<RegiT>::const_iterator iter =
>           loopLand->breakOnRegs.begin(),
>         iterEnd = loopLand->breakOnRegs.end(); iter != iterEnd; ++iter) {
> -    CFGTraits::insertCondBranchEnd(dstBlk, AMDGPU::BREAK_LOGICALNZ_i32, passRep,
> +    CFGTraits::insertCondBranchEnd(dstBlk, AMDGPU::PREDICATED_BREAK, passRep,
>                                     *iter);
>    }
>  
> @@ -1630,14 +1630,12 @@ void CFGStructurizer<PassT>::mergeLoopbreakBlock(BlockT *exitingBlk,
>      if (trueBranch != exitBlk) {
>        reversePredicateSetter(branchInstrPos);
>      }
> -    int newOpcode = CFGTraits::getBreakZeroOpcode(oldOpcode);
> -    CFGTraits::insertCondBranchBefore(branchInstrPos, newOpcode, passRep, DL);
> +    CFGTraits::insertCondBranchBefore(branchInstrPos, AMDGPU::PREDICATED_BREAK, passRep, DL);
>    } else {
>      if (trueBranch != exitBlk) {
>        reversePredicateSetter(branchInstr);
>      }
> -    int newOpcode = CFGTraits::getBreakZeroOpcode(oldOpcode);
> -    CFGTraits::insertCondBranchBefore(branchInstrPos, newOpcode, passRep, DL);
> +    CFGTraits::insertCondBranchBefore(branchInstrPos, AMDGPU::PREDICATED_BREAK, passRep, DL);
>      if (exitBlk != exitLandBlk) {
>        //splice is insert-before ...
>        exitingBlk->splice(branchInstrPos, exitBlk, exitBlk->begin(),
> @@ -2696,27 +2694,9 @@ struct CFGStructTraits<AMDGPUCFGStructurizer>
>  {
>    typedef int RegiT;
>  
> -  static int getBreakNzeroOpcode(int oldOpcode) {
> -    switch(oldOpcode) {
> -      case AMDGPU::JUMP: return AMDGPU::BREAK_LOGICALNZ_i32;
> -    default:
> -      assert(0 && "internal error");
> -    };
> -    return -1;
> -  }
> -
> -  static int getBreakZeroOpcode(int oldOpcode) {
> -    switch(oldOpcode) {
> -      case AMDGPU::JUMP: return AMDGPU::BREAK_LOGICALZ_i32;
> -    default:
> -      assert(0 && "internal error");
> -    };
> -    return -1;
> -  }
> -
>    static int getBranchNzeroOpcode(int oldOpcode) {
>      switch(oldOpcode) {
> -    case AMDGPU::JUMP: return AMDGPU::IF_LOGICALNZ_i32;
> +    case AMDGPU::JUMP: return AMDGPU::IF_PREDICATE_SET;
>        ExpandCaseToAllScalarReturn(AMDGPU::BRANCH_COND, AMDGPU::IF_LOGICALNZ);
>        case AMDGPU::SI_IF_NZ: return AMDGPU::SI_IF_NZ;
>      default:
> @@ -2727,7 +2707,7 @@ struct CFGStructTraits<AMDGPUCFGStructurizer>
>  
>    static int getBranchZeroOpcode(int oldOpcode) {
>      switch(oldOpcode) {
> -    case AMDGPU::JUMP: return AMDGPU::IF_LOGICALZ_i32;
> +    case AMDGPU::JUMP: return AMDGPU::IF_PREDICATE_SET;
>        ExpandCaseToAllScalarReturn(AMDGPU::BRANCH_COND, AMDGPU::IF_LOGICALZ);
>        case AMDGPU::SI_IF_Z: return AMDGPU::SI_IF_Z;
>      default:
> @@ -2874,7 +2854,7 @@ struct CFGStructTraits<AMDGPUCFGStructurizer>
>    static MachineInstr *getLoopBreakInstr(MachineBasicBlock *blk) {
>      for (MachineBasicBlock::iterator iter = blk->begin(); (iter != blk->end()); ++iter) {
>        MachineInstr *instr = &(*iter);
> -      if ((instr->getOpcode() == AMDGPU::BREAK_LOGICALNZ_i32) || (instr->getOpcode() == AMDGPU::BREAK_LOGICALZ_i32)) {
> +      if (instr->getOpcode() == AMDGPU::PREDICATED_BREAK) {
>          return instr;
>        }
>      }
> diff --git a/lib/Target/AMDGPU/MCTargetDesc/R600MCCodeEmitter.cpp b/lib/Target/AMDGPU/MCTargetDesc/R600MCCodeEmitter.cpp
> index 636389d..21b639b 100644
> --- a/lib/Target/AMDGPU/MCTargetDesc/R600MCCodeEmitter.cpp
> +++ b/lib/Target/AMDGPU/MCTargetDesc/R600MCCodeEmitter.cpp
> @@ -106,17 +106,13 @@ enum InstrTypes {
>  };
>  
>  enum FCInstr {
> -  FC_IF = 0,
> -  FC_IF_INT,
> +  FC_IF_PREDICATE = 0,
>    FC_ELSE,
>    FC_ENDIF,
>    FC_BGNLOOP,
>    FC_ENDLOOP,
> -  FC_BREAK,
> -  FC_BREAK_NZ_INT,
> -  FC_CONTINUE,
> -  FC_BREAK_Z_INT,
> -  FC_BREAK_NZ
> +  FC_BREAK_PREDICATE,
> +  FC_CONTINUE
>  };
>  
>  enum TextureTypes {
> @@ -442,28 +438,14 @@ void R600MCCodeEmitter::EmitFCInstr(const MCInst &MI, raw_ostream &OS) const {
>    // Emit FC Instruction
>    enum FCInstr instr;
>    switch (MI.getOpcode()) {
> -  case AMDGPU::BREAK_LOGICALZ_f32:
> -    instr = FC_BREAK;
> -    break;
> -  case AMDGPU::BREAK_LOGICALNZ_f32:
> -    instr = FC_BREAK_NZ;
> -    break;
> -  case AMDGPU::BREAK_LOGICALNZ_i32:
> -    instr = FC_BREAK_NZ_INT;
> -    break;
> -  case AMDGPU::BREAK_LOGICALZ_i32:
> -    instr = FC_BREAK_Z_INT;
> +  case AMDGPU::PREDICATED_BREAK:
> +    instr = FC_BREAK_PREDICATE;
>      break;
>    case AMDGPU::CONTINUE:
>      instr = FC_CONTINUE;
>      break;
> -  case AMDGPU::IF_LOGICALNZ_f32:
> -    instr = FC_IF;
> -  case AMDGPU::IF_LOGICALNZ_i32:
> -    instr = FC_IF_INT;
> -    break;
> -  case AMDGPU::IF_LOGICALZ_f32:
> -    abort();
> +  case AMDGPU::IF_PREDICATE_SET:
> +    instr = FC_IF_PREDICATE;
>      break;
>    case AMDGPU::ELSE:
>      instr = FC_ELSE;
> @@ -546,17 +528,12 @@ uint64_t R600MCCodeEmitter::getMachineOpValue(const MCInst &MI,
>  bool R600MCCodeEmitter::isFCOp(unsigned opcode) const {
>    switch(opcode) {
>    default: return false;
> -  case AMDGPU::BREAK_LOGICALZ_f32:
> -  case AMDGPU::BREAK_LOGICALNZ_i32:
> -  case AMDGPU::BREAK_LOGICALZ_i32:
> -  case AMDGPU::BREAK_LOGICALNZ_f32:
> +  case AMDGPU::PREDICATED_BREAK:
>    case AMDGPU::CONTINUE:
> -  case AMDGPU::IF_LOGICALNZ_i32:
> -  case AMDGPU::IF_LOGICALZ_f32:
> +  case AMDGPU::IF_PREDICATE_SET:
>    case AMDGPU::ELSE:
>    case AMDGPU::ENDIF:
>    case AMDGPU::ENDLOOP:
> -  case AMDGPU::IF_LOGICALNZ_f32:
>    case AMDGPU::WHILELOOP:
>      return true;
>    }
> diff --git a/lib/Target/AMDGPU/R600ExpandSpecialInstrs.cpp b/lib/Target/AMDGPU/R600ExpandSpecialInstrs.cpp
> index e040e4c..3cfc752 100644
> --- a/lib/Target/AMDGPU/R600ExpandSpecialInstrs.cpp
> +++ b/lib/Target/AMDGPU/R600ExpandSpecialInstrs.cpp
> @@ -209,7 +209,7 @@ bool R600ExpandSpecialInstrsPass::runOnMachineFunction(MachineFunction &MF) {
>          TII->setImmOperand(PredSet, R600Operands::UPDATE_EXEC_MASK, 1);
>  
>          BuildMI(MBB, I, MBB.findDebugLoc(I),
> -                TII->get(AMDGPU::BREAK_LOGICALNZ_i32))
> +                TII->get(AMDGPU::PREDICATED_BREAK))
>                  .addReg(AMDGPU::PREDICATE_BIT);
>          MI.eraseFromParent();
>          continue;
> diff --git a/lib/Target/AMDGPU/R600Instructions.td b/lib/Target/AMDGPU/R600Instructions.td
> index 458d5b7..b0c1628 100644
> --- a/lib/Target/AMDGPU/R600Instructions.td
> +++ b/lib/Target/AMDGPU/R600Instructions.td
> @@ -1456,6 +1456,17 @@ def : Pat<(fsqrt R600_Reg32:$src),
>  } // End isCayman
>  
>  //===----------------------------------------------------------------------===//
> +// Branch Instructions
> +//===----------------------------------------------------------------------===//
> +
> +
> +def IF_PREDICATE_SET  : ILFormat<(outs), (ins GPRI32:$src),
> +  "IF_PREDICATE_SET $src", []>;
> +
> +def PREDICATED_BREAK : ILFormat<(outs), (ins GPRI32:$src),
> +  "PREDICATED_BREAK $src", []>;
> +
> +//===----------------------------------------------------------------------===//
>  // Pseudo instructions
>  //===----------------------------------------------------------------------===//
>  
> -- 
> 1.8.0
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list