[llvm] r274193 - CodeGen: Add an explicit BuildMI overload for MachineInstr&

[Justin Bogner via llvm-commits]

"Duncan P. N. Exon Smith via llvm-commits" <llvm-commits at lists.llvm.org>
writes:
> Author: dexonsmith
> Date: Wed Jun 29 19:10:17 2016
> New Revision: 274193
>
> URL: http://llvm.org/viewvc/llvm-project?rev=274193&view=rev
> Log:
> CodeGen: Add an explicit BuildMI overload for MachineInstr&
>
> Add an explicit overload to BuildMI for MachineInstr& to deal with
> insertions inside of instruction bundles.

You missed the other BuildMI MachineInstr* overload. I've added it in
r274259 following the same pattern as here.

> - Use it to re-implement MachineInstr* to give it coverage.
>
> - Document how the overload for MachineBasicBlock::instr_iterator
>   differs from that for MachineBasicBlock::iterator (the previous
>   (implicit) overload for MachineInstr&).
>
> - Add a comment explaining why the MachineInstr& and MachineInstr*
>   overloads don't universally forward to the
>   MachineBasicBlock::instr_iterator overload.
>
> Thanks to Justin for noticing the API quirk.  While this doesn't fix any
> known bugs -- all uses of BuildMI with a MachineInstr& were previously
> using MachineBasicBlock::iterator -- it protects against future bugs.
>
> Modified:
>     llvm/trunk/include/llvm/CodeGen/MachineInstrBuilder.h
>
> Modified: llvm/trunk/include/llvm/CodeGen/MachineInstrBuilder.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineInstrBuilder.h?rev=274193&r1=274192&r2=274193&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/MachineInstrBuilder.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/MachineInstrBuilder.h Wed Jun 29 19:10:17 2016
> @@ -265,6 +265,12 @@ inline MachineInstrBuilder BuildMI(Machi
>    return MachineInstrBuilder(MF, MI).addReg(DestReg, RegState::Define);
>  }
>  
> +/// This version of the builder inserts the newly-built instruction before
> +/// the given position in the given MachineBasicBlock, and sets up the first
> +/// operand as a destination virtual register.
> +///
> +/// If \c I is inside a bundle, then the newly inserted \a MachineInstr is
> +/// added to the same bundle.
>  inline MachineInstrBuilder BuildMI(MachineBasicBlock &BB,
>                                     MachineBasicBlock::instr_iterator I,
>                                     const DebugLoc &DL, const MCInstrDesc &MCID,
> @@ -275,16 +281,20 @@ inline MachineInstrBuilder BuildMI(Machi
>    return MachineInstrBuilder(MF, MI).addReg(DestReg, RegState::Define);
>  }
>  
> -inline MachineInstrBuilder BuildMI(MachineBasicBlock &BB, MachineInstr *I,
> +inline MachineInstrBuilder BuildMI(MachineBasicBlock &BB, MachineInstr &I,
>                                     const DebugLoc &DL, const MCInstrDesc &MCID,
>                                     unsigned DestReg) {
> -  if (I->isInsideBundle()) {
> -    MachineBasicBlock::instr_iterator MII(I);
> -    return BuildMI(BB, MII, DL, MCID, DestReg);
> -  }
> +  // Calling the overload for instr_iterator is always correct.  However, the
> +  // definition is not available in headers, so inline the check.
> +  if (I.isInsideBundle())
> +    return BuildMI(BB, MachineBasicBlock::instr_iterator(I), DL, MCID, DestReg);
> +  return BuildMI(BB, MachineBasicBlock::iterator(I), DL, MCID, DestReg);
> +}
>  
> -  MachineBasicBlock::iterator MII = I;
> -  return BuildMI(BB, MII, DL, MCID, DestReg);
> +inline MachineInstrBuilder BuildMI(MachineBasicBlock &BB, MachineInstr *I,
> +                                   const DebugLoc &DL, const MCInstrDesc &MCID,
> +                                   unsigned DestReg) {
> +  return BuildMI(BB, *I, DL, MCID, DestReg);
>  }
>  
>  /// This version of the builder inserts the newly-built instruction before the
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits