[PATCH] D115208: [NFC][MachineInstr] No need to std::move the DebugLoc

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added a comment.

In D115208#3175303 <https://reviews.llvm.org/D115208#3175303>, @dblaikie wrote:

> I /think/ given that `DebugLoc` has a `TrackingMDNodeRef` member, which has a `TrackingMDRef` member which has cheaper move than copy, then DebugLoc /probably/ should be passed by value when a copy is going to be made - since it means callers can pass by lvalue or rvalue and get move opportunities when possible.
>
> Given this API scenario:
>
>   CheapToMove Dest;
>   void Caller() {
>     CheapToMove Source;
>     Intermediate(Source); // 1 
>     Intermediate(CheapToMove()); //2
>   }
>   void Implementation(/* What Type Goes here? */ CTM) {
>     Dest = std::move(CTM); // move is a no-op here if CTM hapens to be a const ref
>   }
>
> If the type is const ref, then (1) produces a single copy and (2) produces a single copy.
> If the type is by-value, then (1) produces a move and a copy (worse by a copy, that should be cheap), and (2) produces 2 moves (better for cheaply movable things).
>
> Does that make sense?

Ah, so you're saying MachineFunction::CreateMachineInstr should instead accept its DebugLoc by value - leaving the rest unchanged (i.e. revert this patch and change CreateMachineInstr instead)?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115208/new/

https://reviews.llvm.org/D115208