[MSP430] Recognize Bi as an indirect branch in analyzeBranch. NFC.

[foad]

Summary

Recognize Bi as an unconditional branch, just like JMP. This allows
machine verification to run after MSP430BranchSelector without failing
this assertion:

virtual bool llvm::MSP430InstrInfo::analyzeBranch(llvm::MachineBasicBlock &, llvm::MachineBasicBlock *&, llvm::MachineBasicBlock *&, SmallVectorImpl<llvm::MachineOperand> &, bool) const: Assertion `I->getOpcode() == MSP430::JCC && "Invalid conditional branch"' failed.

Note that machine verification is currently disabled after
addPreEmitPass passes because of problems on other targets, so this is
currently NFC.

[jozefl]

Instead of just ignoring Bi, I think we can handle it as we do with JMP. Bi is a direct, unconditional branch like JMP, it just has longer range.
See inline comments for suggestions.

Thanks!

llvm/lib/Target/MSP430/MSP430InstrInfo.cpp

118–119

MSP430::Bi should be added here as well

193

[foad]

OK, I will try your suggestions, but:

Bi is a direct, unconditional branch like JMP

then why does the Bi instruction have "isIndirectBranch = 1"?

[foad]

Treat Bi as an unconditional branch.

[jozefl]

In D110691#3029803, @foad wrote:

OK, I will try your suggestions, but:

Bi is a direct, unconditional branch like JMP

then why does the Bi instruction have "isIndirectBranch = 1"?

I'm not sure, looks like an oversight to me. I'll make a separate patch to fix that.

Bi corresponds to BR #imm. BR #imm is emulated as MOV @PC+, PC, which may appear to be indirect, but this is just how MSP430 deals with immediate values; the immediate value is stored in the next word after the opcode.
I'm guessing analyzeBranch cannot handle indirect branches because the destination basic block of the branch is not known when the branch target is stored in a register or memory address, but for Bi, the destination BB is known.

So this patch looks good to me, but I would like to give @asl a chance to have a look. Since this is NFC you can probably go ahead and commit it if there's no further activity after 24 hours.

Thanks,
Jozef

[asl]

Indeed, this was an oversight. Bi could be generated from an indirect branch as a result of folding or during the branch relaxation.

Interesting enough, whether we could drop branch selector pass in favour of generic BranchRelaxation pass? (MSP430 branch selector dates back to 2010 where no generic branch relaxation pass was available, the latter was created our of AArch64 one).

llvm/lib/Target/MSP430/MSP430InstrInfo.cpp

unsigned MSP430InstrInfo::removeBranch(MachineBasicBlock &MBB,
 
   MachineBasicBlock::iterator I = MBB.end();
   unsigned Count = 0;
 
   while (I != MBB.begin()) {
     --I;
     if (I->isDebugInstr())
       continue;
     if (I->getOpcode() != MSP430::JMP &&
         I->getOpcode() != MSP430::JCC &&
+        I->getOpcode() != MSP430::Bi &&
         I->getOpcode() != MSP430::Br &&
         I->getOpcode() != MSP430::Bm)
       break;
     // Remove the branch.
     I->eraseFromParent();
     I = MBB.end();
     ++Count;
   }
if (!I->isBranch())
       return true;
 
     // Cannot handle indirect branches.
     if (I->getOpcode() == MSP430::Br ||
         I->getOpcode() == MSP430::Bm)
       return true;
 
     // Handle unconditional branches.
-    if (I->getOpcode() == MSP430::JMP) {
+    if (I->getOpcode() == MSP430::JMP || I->getOpcode() == MSP430::Bi) {
       if (!AllowModify) {
         TBB = I->getOperand(0).getMBB();
         continue;
       }
 
       // If the block has any instructions after a JMP, delete them.
       while (std::next(I) != MBB.end())
         std::next(I)->eraseFromParent();