[VE] Clean check routines of branch types

[kaz7]

Summary

Previously, these check routines accepted non-generatble instructions.
This time, I clean them and add assert for those non-generatable
instructions.

[simoll]

A helpful comment could explain why these instructions should not be used. Nits, else LGTM.

llvm/lib/Target/VE/VEInstrInfo.cpp

97

typo alwasy

101

I am lacking context here but is this really the right place for this assertion? Does "should not be used" mean that these instructions are invalid/incorrect or are they just not recommendable/inefficient?

[kaz7]

Thank you for reviewing.

llvm/lib/Target/VE/VEInstrInfo.cpp

97

Thanks.

101

Those instructions are not lowered here from LLVM-IR because we don't use such instructions in our lowering patterns. Those are acceptable by meaning of HW instructions but not acceptable by meaning of lowering consitency. And, writing all possible instruction here is definitely not a good idea. Therefore, I write "should not be used" as invalid. I'll write such comments in the souce.

[kaz7]

Update comments following suggestions.

[simoll]

Thanks!

llvm/lib/Target/VE/VEInstrInfo.cpp

static VECC::CondCode GetOppositeBranchCondition(VECC::CondCode CC) {
   case VECC::CC_AT:
     return VECC::CC_AF;
   case VECC::UNKNOWN:
     return VECC::UNKNOWN;
   }
   llvm_unreachable("Invalid cond code");
 }
 
-// Treat br.l [BRCF AT] as unconditional branch
+// Treat a branch relative long always instruction as unconditional branch.
+// For example, br.l.t and br.l.
 static bool isUncondBranchOpcode(int Opc) {
-  return Opc == VE::BRCFLa    || Opc == VE::BRCFWa    ||
-         Opc == VE::BRCFLa_nt || Opc == VE::BRCFWa_nt ||
-         Opc == VE::BRCFLa_t  || Opc == VE::BRCFWa_t  ||
-         Opc == VE::BRCFDa    || Opc == VE::BRCFSa    ||
-         Opc == VE::BRCFDa_nt || Opc == VE::BRCFSa_nt ||
-         Opc == VE::BRCFDa_t  || Opc == VE::BRCFSa_t;
+  using namespace llvm::VE;
+
+#define BRKIND(NAME) (Opc == NAME##a || Opc == NAME##a_nt || Opc == NAME##a_t)
+  // VE has other branch relative always instructions for word/double/float,
+  // but we use only long branches in our lower.  So, sanity check it here.
+  assert(!BRKIND(BRCFW) && !BRKIND(BRCFD) && !BRKIND(BRCFS) &&
+         "Branch relative word/double/float always instructions should not be "
+         "used!");
+  return BRKIND(BRCFL);
+#undef BRKIND
 }
 
+// Treat branch relative conditional as conditional branch instructions.
+// For example, brgt.l.t and brle.s.nt.
 static bool isCondBranchOpcode(int Opc) {
-  return Opc == VE::BRCFLrr    || Opc == VE::BRCFLir    ||
-         Opc == VE::BRCFLrr_nt || Opc == VE::BRCFLir_nt ||
-         Opc == VE::BRCFLrr_t  || Opc == VE::BRCFLir_t  ||
-         Opc == VE::BRCFWrr    || Opc == VE::BRCFWir    ||
-         Opc == VE::BRCFWrr_nt || Opc == VE::BRCFWir_nt ||
-         Opc == VE::BRCFWrr_t  || Opc == VE::BRCFWir_t  ||
-         Opc == VE::BRCFDrr    || Opc == VE::BRCFDir    ||
-         Opc == VE::BRCFDrr_nt || Opc == VE::BRCFDir_nt ||
-         Opc == VE::BRCFDrr_t  || Opc == VE::BRCFDir_t  ||
-         Opc == VE::BRCFSrr    || Opc == VE::BRCFSir    ||
-         Opc == VE::BRCFSrr_nt || Opc == VE::BRCFSir_nt ||
-         Opc == VE::BRCFSrr_t  || Opc == VE::BRCFSir_t;
+  using namespace llvm::VE;
+
+#define BRKIND(NAME)                                                           \
+  (Opc == NAME##rr || Opc == NAME##rr_nt || Opc == NAME##rr_t ||               \
+   Opc == NAME##ir || Opc == NAME##ir_nt || Opc == NAME##ir_t)
+  return BRKIND(BRCFL) || BRKIND(BRCFW) || BRKIND(BRCFD) || BRKIND(BRCFS);
+#undef BRKIND
 }
 
+// Treat branch long always instructions as indirect branch.
+// For example, b.l.t and b.l.
 static bool isIndirectBranchOpcode(int Opc) {
-  return Opc == VE::BCFLari    || Opc == VE::BCFLari    ||
-         Opc == VE::BCFLari_nt || Opc == VE::BCFLari_nt ||
-         Opc == VE::BCFLari_t  || Opc == VE::BCFLari_t  ||
-         Opc == VE::BCFLari    || Opc == VE::BCFLari    ||
-         Opc == VE::BCFLari_nt || Opc == VE::BCFLari_nt ||
-         Opc == VE::BCFLari_t  || Opc == VE::BCFLari_t;
+  using namespace llvm::VE;
+
+#define BRKIND(NAME)                                                           \
+  (Opc == NAME##ari || Opc == NAME##ari_nt || Opc == NAME##ari_t)
+  // VE has other branch always instructions for word/double/float, but
+  // we use only long branches in our lower.  So, sanity check it here.
+  assert(!BRKIND(BCFW) && !BRKIND(BCFD) && !BRKIND(BCFS) &&
+         "Branch word/double/float always instructions should not be used!");
+  return BRKIND(BCFL);
+#undef BRKIND
 }
 
 static void parseCondBranch(MachineInstr *LastInst, MachineBasicBlock *&Target,
                             SmallVectorImpl<MachineOperand> &Cond) {
   Cond.push_back(MachineOperand::CreateImm(LastInst->getOperand(0).getImm()));
   Cond.push_back(LastInst->getOperand(1));
   Cond.push_back(LastInst->getOperand(2));
   Target = LastInst->getOperand(3).getMBB();

llvm/test/CodeGen/VE/Scalar/br_analyze.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=ve | FileCheck %s
+
+declare void @foo() noreturn
+
+;;; Check a case where a separate branch is needed and where the original
+;;; order should be reversed.  Copied from SystemZ/branch-08.ll
+
+define i32 @f1(i32 %a, i32 *%bptr) {
+; CHECK-LABEL: f1:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    st %s9, (, %s11)
+; CHECK-NEXT:    st %s10, 8(, %s11)
+; CHECK-NEXT:    or %s9, 0, %s11
+; CHECK-NEXT:    lea %s11, -240(, %s11)
+; CHECK-NEXT:    brge.l %s11, %s8, .LBB0_4
+; CHECK-NEXT:  # %bb.3: # %entry
+; CHECK-NEXT:    ld %s61, 24(, %s14)
+; CHECK-NEXT:    or %s62, 0, %s0
+; CHECK-NEXT:    lea %s63, 315
+; CHECK-NEXT:    shm.l %s63, (%s61)
+; CHECK-NEXT:    shm.l %s8, 8(%s61)
+; CHECK-NEXT:    shm.l %s11, 16(%s61)
+; CHECK-NEXT:    monc
+; CHECK-NEXT:    or %s0, 0, %s62
+; CHECK-NEXT:  .LBB0_4: # %entry
+; CHECK-NEXT:    ldl.sx %s1, (, %s1)
+; CHECK-NEXT:    cmpu.w %s0, %s1, %s0
+; CHECK-NEXT:    brlt.w 0, %s0, .LBB0_2
+; CHECK-NEXT:  # %bb.1: # %return
+; CHECK-NEXT:    or %s0, 1, (0)1
+; CHECK-NEXT:    or %s11, 0, %s9
+; CHECK-NEXT:    ld %s10, 8(, %s11)
+; CHECK-NEXT:    ld %s9, (, %s11)
+; CHECK-NEXT:    b.l.t (, %s10)
+; CHECK-NEXT:  .LBB0_2: # %callit
+; CHECK-NEXT:    lea %s0, foo@lo
+; CHECK-NEXT:    and %s0, %s0, (32)0
+; CHECK-NEXT:    lea.sl %s12, foo@hi(, %s0)
+; CHECK-NEXT:    bsic %s10, (, %s12)
+entry:
+  %b = load i32, i32 *%bptr
+  %cmp = icmp ult i32 %a, %b
+  br i1 %cmp, label %callit, label %return
+
+callit:
+  call void @foo()
+  unreachable
+
+return:
+  ret i32 1
+}
+
+;;; Same again with a fused compare and branch.
+
+define i32 @f2(i32 %a) {
+; CHECK-LABEL: f2:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    st %s9, (, %s11)
+; CHECK-NEXT:    st %s10, 8(, %s11)
+; CHECK-NEXT:    or %s9, 0, %s11
+; CHECK-NEXT:    lea %s11, -240(, %s11)
+; CHECK-NEXT:    brge.l %s11, %s8, .LBB1_4
+; CHECK-NEXT:  # %bb.3: # %entry
+; CHECK-NEXT:    ld %s61, 24(, %s14)
+; CHECK-NEXT:    or %s62, 0, %s0
+; CHECK-NEXT:    lea %s63, 315
+; CHECK-NEXT:    shm.l %s63, (%s61)
+; CHECK-NEXT:    shm.l %s8, 8(%s61)
+; CHECK-NEXT:    shm.l %s11, 16(%s61)
+; CHECK-NEXT:    monc
+; CHECK-NEXT:    or %s0, 0, %s62
+; CHECK-NEXT:  .LBB1_4: # %entry
+; CHECK-NEXT:    breq.w 0, %s0, .LBB1_2
+; CHECK-NEXT:  # %bb.1: # %return
+; CHECK-NEXT:    or %s0, 1, (0)1
+; CHECK-NEXT:    or %s11, 0, %s9
+; CHECK-NEXT:    ld %s10, 8(, %s11)
+; CHECK-NEXT:    ld %s9, (, %s11)
+; CHECK-NEXT:    b.l.t (, %s10)
+; CHECK-NEXT:  .LBB1_2: # %callit
+; CHECK-NEXT:    lea %s0, foo@lo
+; CHECK-NEXT:    and %s0, %s0, (32)0
+; CHECK-NEXT:    lea.sl %s12, foo@hi(, %s0)
+; CHECK-NEXT:    bsic %s10, (, %s12)
+entry:
+  %cmp = icmp eq i32 %a, 0
+  br i1 %cmp, label %callit, label %return
+
+callit:
+  call void @foo()
+  unreachable
+
+return:
+  ret i32 1
+}