[mips] Enforce compact branch restrictions

[sdardis]

Summary

Check both operands for use of the $zero register which cannot be used with
an compact branch instruction.

[dsanders]

lib/Target/Mips/MipsInstrInfo.cpp

437–442

There are currently six iterations over the operand list. 5 are findRegisterUseOperandIdx() calls (two of which are inside readsRegister()), then one more to copy the operands. findRegisterUseOperandIdx() checks subregisters so we can cut it down to two with:

int ZeroOperandPosition = -1;
if (I->isBranch() && !I->isPseudo()) {
  // Check for reads of any portion of ZERO_64.
  ZeroOperandPosition = I->findRegisterUseOperandIdx(Mips::ZERO_64);
  BranchWithZeroOperand = ZeroOperandPosition != -1;
}

then add the necessary signed -> unsigned cast in the loop below.

485

indentation

test/CodeGen/Mips/compactbranches/no-beqzc-bnezc.ll

106–107

Could you check for the correct branch instruction instead? It's easy to get false-passes with just CHECK-NOT.

121–122

Could you check for the correct branch instruction instead?

[sdardis]

Updated as per reviewer's comments.

[dsanders]

LGTM with a couple nits

lib/Target/Mips/MipsInstrInfo.cpp

438

I think the 'auto' should be 'const auto &' if possible and 'auto &' otherwise.

439

I think we should include a comment that this will also find ZERO_64

[sdardis]

Thanks, committed rL278824.

lib/Target/Mips/MipsInstrInfo.cpp

MipsInstrInfo::genInstrWithNewOpc(unsigned NewOpc,
 
   // Certain branches have two forms: e.g beq $1, $zero, dest vs beqz $1, dest
   // Pick the zero form of the branch for readable assembly and for greater
   // branch distance in non-microMIPS mode.
   // FIXME: Certain atomic sequences on mips64 generate 32bit references to
   // Mips::ZERO, which is incorrect. This test should be updated to use
   // Subtarget.getABI().GetZeroReg() when those atomic sequences and others
   // are fixed.
+  unsigned ZeroOperandPosition = 0;
   bool BranchWithZeroOperand =
-      (I->isBranch() && !I->isPseudo() && I->getOperand(1).isReg() &&
-       (I->getOperand(1).getReg() == Mips::ZERO ||
-        I->getOperand(1).getReg() == Mips::ZERO_64));
+      I->isBranch() && !I->isPseudo() &&
+      (I->readsRegister(Mips::ZERO) || I->readsRegister(Mips::ZERO_64));
+  if (BranchWithZeroOperand) {
+    ZeroOperandPosition =
+        I->findRegisterUseOperandIdx(Mips::ZERO) == -1
+            ? (unsigned)I->findRegisterUseOperandIdx(Mips::ZERO_64)
+            : (unsigned)I->findRegisterUseOperandIdx(Mips::ZERO);
+  }
 
   if (BranchWithZeroOperand) {
     switch (NewOpc) {
     case Mips::BEQC:
       NewOpc = Mips::BEQZC;
       break;
     case Mips::BNEC:
       NewOpc = Mips::BNEZC;
if (NewOpc == Mips::JIALC || NewOpc == Mips::JIALC64)
       MIB->RemoveOperand(0);
 
     for (unsigned J = 0, E = I->getDesc().getNumOperands(); J < E; ++J) {
       MIB.addOperand(I->getOperand(J));
     }
 
     MIB.addImm(0);
 
- } else if (BranchWithZeroOperand) {
-    // For MIPSR6 and microMIPS branches with an explicit zero operand, copy
-    // everything after the zero.
-     MIB.addOperand(I->getOperand(0));
-
-    for (unsigned J = 2, E = I->getDesc().getNumOperands(); J < E; ++J) {
-      MIB.addOperand(I->getOperand(J));
-    }
-  } else {
-    // All other cases copy all other operands.
+
+    // For MIPSR6 and microMIPS branches with an explicit zero operand, skip
+    // copying the zero.
     for (unsigned J = 0, E = I->getDesc().getNumOperands(); J < E; ++J) {
+      if (BranchWithZeroOperand && ZeroOperandPosition == J)
+        continue;
+
       MIB.addOperand(I->getOperand(J));
     }
   }
 
   MIB.copyImplicitOps(*I);
 
   MIB.setMemRefs(I->memoperands_begin(), I->memoperands_end());
   return MIB;
 }

test/CodeGen/Mips/compactbranches/no-beqzc-bnezc.ll

; ENCODING-NOT:   bnec $5, $4
   br i1 %cmp, label %if.then, label %if.end
 
   if.then:
     ret i64 1
 
   if.end:
     ret i64 0
 }
+
+define i32 @f6(i32 %a) {
+; CHECK-LABEL: f6:
+; CHECK-NOT: beqc ${{[0-9]+}}, $zero, $BB[0-9]+_[0-9]+
+; CHECK-NOT: bnec ${{[0-9]+}}, $zero, $BB[0-9]+_[0-9]+
+
+  %cmp = icmp eq i32 %a, 0
+  br i1 %cmp, label %if.then, label %if.end
+
+  if.then:
+    ret i32 1
+
+  if.end:
+    ret i32 0
+}
+
+define i32 @f7(i32 %a) {
+; CHECK-LABEL: f7:
+; CHECK-NOT: beqc $zero, ${{[0-9]+}}, $BB[0-9]+_[0-9]+
+; CHECK-NOT: bnec $zero, ${{[0-9]+}}, $BB[0-9]+_[0-9]+
+
+  %cmp = icmp eq i32 0, %a
+  br i1 %cmp, label %if.then, label %if.end
+
+  if.then:
+    ret i32 1
+
+  if.end:
+    ret i32 0
+}