[PATCH] D48019: [mips] Handle branch expansion corner cases

Aleksandar Beserminji via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 18 00:18:36 PDT 2018


abeserminji added inline comments.


================
Comment at: lib/Target/Mips/MipsBranchExpansion.cpp:392
+
+  return hasDelaySlot;
+}
----------------
atanasyan wrote:
> It looks like `addImm` and `hasDelaySlot` always have different values. Maybe use, for example, the first one and return from the function `!addImm`?
> 
> BTW what do you think about the code below? I think it's more clear represent selection of the jump operation. But I do not force this variant.
> 
> 
> ```
> bool MipsBranchExpansion::buildProperJumpMI(MachineBasicBlock *MBB,
>                                             MachineBasicBlock::iterator Pos,
>                                             DebugLoc DL) {
>   bool HasR6 = ABI.IsN64() ? STI->hasMips64r6() : STI->hasMips32r6();
>   bool AddImm = HasR6 && !STI->useIndirectJumpsHazard();
> 
>   unsigned JR = ABI.IsN64() ? Mips::JR64 : Mips::JR;
>   unsigned JIC = ABI.IsN64() ? Mips::JIC64 : Mips::JIC;
>   unsigned JR_HB = ABI.IsN64() ? Mips::JR_HB64 : Mips::JR_HB;
>   unsigned JR_HB_R6 = ABI.IsN64() ? Mips::JR_HB64_R6 : Mips::JR_HB_R6;
> 
>   unsigned JumpOp;
>   if (STI->useIndirectJumpsHazard())
>     JumpOp = HasR6 ? JR_HB_R6 : JR_HB;
>   else
>     JumpOp = HasR6 ? JIC : JR;
> 
>   if (JumpOp == Mips::JIC && STI->inMicroMipsMode())
>     JumpOp = Mips::JIC_MMR6;
> 
>   unsigned ATReg = ABI.IsN64() ? Mips::AT_64 : Mips::AT;
>   MachineInstrBuilder Instr =
>       BuildMI(*MBB, Pos, DL, TII->get(JumpOp)).addReg(ATReg);
>   if (AddImm)
>     Instr.addImm(0);
> 
>   return !AddImm;
> }
> ```
I've checked your solution and it looks nicer to me. So I am gonna include it.


https://reviews.llvm.org/D48019





More information about the llvm-commits mailing list