[PATCH] [mips] [IAS] Add support for the B{L, G}{T, E}(U) branch pseudo-instructions.

Daniel Sanders daniel.sanders at imgtec.com
Fri Jun 12 02:50:09 PDT 2015


LGTM with a style nit.

One thing that we will need to address soon is that my while earlier test produces the same results for clang+IAS and gcc+GAS but clang+GAS produces completely different output. Presumably there's a clang-driver bug. Still, IAS is doing the right thing so this patch is good.

Also just an interesting observation, GAS only emits three 'branch ... is always true' warnings whereas IAS emits six. It seems GAS doesn't recognise $0 >= $0 and similar as being always true. IAS is doing the right thing here.


================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:2308-2441
@@ +2307,136 @@
+  bool IsSrcRegZero = (SrcReg == Mips::ZERO);
+  if (IsSrcRegZero && IsTrgRegZero) {
+    // FIXME: All of these Opcode-specific if's are needed for compatibility
+    // with GAS' behaviour. However, they may not generate the most efficient
+    // code in some circumstances.
+    if (PseudoOpcode == Mips::BLT) {
+      BranchInst.setOpcode(Mips::BLTZ);
+      BranchInst.addOperand(MCOperand::createReg(Mips::ZERO));
+      BranchInst.addOperand(MCOperand::createExpr(OffsetExpr));
+      Instructions.push_back(BranchInst);
+    } else if (PseudoOpcode == Mips::BLE) {
+      BranchInst.setOpcode(Mips::BLEZ);
+      BranchInst.addOperand(MCOperand::createReg(Mips::ZERO));
+      BranchInst.addOperand(MCOperand::createExpr(OffsetExpr));
+      Instructions.push_back(BranchInst);
+      Warning(IDLoc, "branch is always taken");
+    } else if (PseudoOpcode == Mips::BGE) {
+      BranchInst.setOpcode(Mips::BGEZ);
+      BranchInst.addOperand(MCOperand::createReg(Mips::ZERO));
+      BranchInst.addOperand(MCOperand::createExpr(OffsetExpr));
+      Instructions.push_back(BranchInst);
+      Warning(IDLoc, "branch is always taken");
+    } else if (PseudoOpcode == Mips::BGT) {
+      BranchInst.setOpcode(Mips::BGTZ);
+      BranchInst.addOperand(MCOperand::createReg(Mips::ZERO));
+      BranchInst.addOperand(MCOperand::createExpr(OffsetExpr));
+      Instructions.push_back(BranchInst);
+    } else if (PseudoOpcode == Mips::BGTU) {
+      BranchInst.setOpcode(Mips::BNE);
+      BranchInst.addOperand(MCOperand::createReg(Mips::ZERO));
+      BranchInst.addOperand(MCOperand::createReg(Mips::ZERO));
+      BranchInst.addOperand(MCOperand::createExpr(OffsetExpr));
+      Instructions.push_back(BranchInst);
+    } else if (AcceptsEquality) {
+      // If both registers are $0 and the pseudo-branch accepts equality, it
+      // will always be taken, so we emit an unconditional branch.
+      BranchInst.setOpcode(Mips::BEQ);
+      BranchInst.addOperand(MCOperand::createReg(Mips::ZERO));
+      BranchInst.addOperand(MCOperand::createReg(Mips::ZERO));
+      BranchInst.addOperand(MCOperand::createExpr(OffsetExpr));
+      Instructions.push_back(BranchInst);
+      Warning(IDLoc, "branch is always taken");
+    } else {
+      // If both registers are $0 and the pseudo-branch does not accept
+      // equality, it will never be taken, so we don't have to emit anything.
+      return false;
+    }
+  } else if (IsSrcRegZero || IsTrgRegZero) {
+    if ((IsSrcRegZero && PseudoOpcode == Mips::BGTU) ||
+        (IsTrgRegZero && PseudoOpcode == Mips::BLTU)) {
+      // If the $rs is $0 and the pseudo-branch is BGTU (0 > x) or
+      // if the $rt is $0 and the pseudo-branch is BLTU (x < 0),
+      // the pseudo-branch will never be taken, so we don't emit anything.
+      // This only applies to unsigned pseudo-branches.
+      return false;
+    } else if ((IsSrcRegZero && PseudoOpcode == Mips::BLEU) ||
+               (IsTrgRegZero && PseudoOpcode == Mips::BGEU)) {
+      // If the $rs is $0 and the pseudo-branch is BLEU (0 <= x) or
+      // if the $rt is $0 and the pseudo-branch is BGEU (x >= 0),
+      // the pseudo-branch will always be taken, so we emit an unconditional
+      // branch.
+      // This only applies to unsigned pseudo-branches.
+      BranchInst.setOpcode(Mips::BEQ);
+      BranchInst.addOperand(MCOperand::createReg(Mips::ZERO));
+      BranchInst.addOperand(MCOperand::createReg(Mips::ZERO));
+      BranchInst.addOperand(MCOperand::createExpr(OffsetExpr));
+      Instructions.push_back(BranchInst);
+      Warning(IDLoc, "branch is always taken");
+    } else {
+      if (IsUnsigned) {
+        // If the $rs is $0 and the pseudo-branch is BLTU (0 < x) or
+        // if the $rt is $0 and the pseudo-branch is BGTU (x > 0),
+        // the pseudo-branch will be taken only when the non-zero register is
+        // different from 0, so we emit a BNEZ.
+        //
+        // If the $rs is $0 and the pseudo-branch is BGEU (0 >= x) or
+        // if the $rt is $0 and the pseudo-branch is BLEU (x <= 0),
+        // the pseudo-branch will be taken only when the non-zero register is
+        // equal to 0, so we emit a BEQZ.
+        //
+        // Because only BLEU and BGEU branch on equality, we can use the
+        // AcceptsEquality variable to decide when to emit the BEQZ.
+        BranchInst.setOpcode(AcceptsEquality ? Mips::BEQ : Mips::BNE);
+        BranchInst.addOperand(
+            MCOperand::createReg(IsSrcRegZero ? TrgReg : SrcReg));
+        BranchInst.addOperand(MCOperand::createReg(Mips::ZERO));
+        BranchInst.addOperand(MCOperand::createExpr(OffsetExpr));
+      } else {
+        // If we have a signed pseudo-branch and one of the registers is $0,
+        // we can use an appropriate compare-to-zero branch. We select which one
+        // to use in the switch statement above.
+        BranchInst.setOpcode(IsSrcRegZero ? ZeroSrcOpcode : ZeroTrgOpcode);
+        BranchInst.addOperand(
+            MCOperand::createReg(IsSrcRegZero ? TrgReg : SrcReg));
+        BranchInst.addOperand(MCOperand::createExpr(OffsetExpr));
+      }
+      Instructions.push_back(BranchInst);
+    }
+  } else {
+    // At this point we need AT to perform the expansions.
+    // If it is not available, getATReg() gives an error.
+    unsigned ATRegNum = getATReg(IDLoc);
+    if (!ATRegNum)
+      return true;
+
+    warnIfNoMacro(IDLoc);
+
+    // SLT fits well with 2 of our 4 pseudo-branches:
+    //   BLT, where $rs < $rt, translates into "slt $at, $rs, $rt" and
+    //   BGT, where $rs > $rt, translates into "slt $at, $rt, $rs".
+    // If the result of the SLT is 1, we branch, and if it's 0, we don't.
+    // This is accomplished by using a BNEZ with the result of the SLT.
+    //
+    // The other 2 pseudo-branches are opposites of the above 2 (BGE with BLT
+    // and BLE with BGT), so we change the BNEZ into a a BEQZ.
+    // Because only BGE and BLE branch on equality, we can use the
+    // AcceptsEquality variable to decide when to emit the BEQZ.
+    // Note that the order of the SLT arguments doesn't change between
+    // opposites.
+    //
+    // The same applies to the unsigned variants, except that SLTu is used
+    // instead of SLT.
+    MCInst SetInst;
+    SetInst.setOpcode(IsUnsigned ? Mips::SLTu : Mips::SLT);
+    SetInst.addOperand(MCOperand::createReg(ATRegNum));
+    SetInst.addOperand(MCOperand::createReg(ReverseOrderSLT ? TrgReg : SrcReg));
+    SetInst.addOperand(MCOperand::createReg(ReverseOrderSLT ? SrcReg : TrgReg));
+    Instructions.push_back(SetInst);
+
+    BranchInst.setOpcode(AcceptsEquality ? Mips::BEQ : Mips::BNE);
+    BranchInst.addOperand(MCOperand::createReg(ATRegNum));
+    BranchInst.addOperand(MCOperand::createReg(Mips::ZERO));
+    BranchInst.addOperand(MCOperand::createExpr(OffsetExpr));
+    Instructions.push_back(BranchInst);
+  }
+  return false;
----------------
Nit: Use early-return.

Rather than:
  if (...) {
    ...
  } else if (...) {
    ...
  } else {
    ...
  }
  return false;
Use:
  if (...) {
    ...
    return false;
  }
  if (...) {
    ...
    return false;
  }
  ...
  return false;

http://reviews.llvm.org/D8537

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list