[PATCH] D55253: [RISCV] Fix incorrect use of MCInstBuilder in RISCVMCCodeEmitter

[Roger Ferrer Ibanez via Phabricator via llvm-commits]

rogfer01 added a comment.

Hi, thanks for the patch. I don't think this usage is wrong so we don't have to change it.

`MCInstBuilder` has an implicit conversion to `MCInst&`, but the referenced object does not outlive the object of the `MCInstBuilder` (i.e. `MCInstBuilder` owns that `MCInst`). So it would be an error to bind the returned reference in a way that is used after the owning object (`MCInstBuilder`) dies. This is exactly the mistake I did in https://reviews.llvm.org/rL339654 (note that there, `AUIPC` and `ADDI` are references, if I had not used a reference I would have worked on copies instead and the original code would have been OK too).

In this case, though, we're not binding any referenced to the `MCInst` owned by `MCInstBuilder`. Instead we're assigning to `TmpInst`. This involves the default copy-assignment operator of `MCInst` so the overall usage is fine because we work on a copy.

I think the code is more readable by using `TmpInst` because avoids us to propagate upwards the call to `getBinaryCodeForInstr`.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55253