[RISCV] Fix incorrect use of MCInstBuilder in RISCVMCCodeEmitter

[shiva0217]

Summary

As Roger pointed out in https://reviews.llvm.org/rL339654, MCInst return by MCInstBuilder has lifetime limitation.

[rogfer01]

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.

[shiva0217]

Hi @rogfer01, got it. Thanks to pointing out the difference. I'll drop the patch.

lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp

 // won't go through RISCV MC to MC compressed instruction transformation. This
 // is acceptable because AUIPC has no 16-bit form and C_JALR have no immediate
 // operand field.  We let linker relaxation deal with it. When linker
 // relaxation enabled, AUIPC and JALR have chance relax to JAL. If C extension
 // is enabled, JAL has chance relax to C_JAL.
 void RISCVMCCodeEmitter::expandFunctionCall(const MCInst &MI, raw_ostream &OS,
                                             SmallVectorImpl<MCFixup> &Fixups,
                                             const MCSubtargetInfo &STI) const {
-  MCInst TmpInst;
   MCOperand Func = MI.getOperand(0);
   unsigned Ra = (MI.getOpcode() == RISCV::PseudoTAIL) ? RISCV::X6 : RISCV::X1;
   uint32_t Binary;
 
   assert(Func.isExpr() && "Expected expression");
 
   const MCExpr *Expr = Func.getExpr();
 
   // Create function call expression CallExpr for AUIPC.
   const MCExpr *CallExpr =
       RISCVMCExpr::create(Expr, RISCVMCExpr::VK_RISCV_CALL, Ctx);
 
   // Emit AUIPC Ra, Func with R_RISCV_CALL relocation type.
-  TmpInst = MCInstBuilder(RISCV::AUIPC)
+  Binary =
+      getBinaryCodeForInstr(MCInstBuilder(RISCV::AUIPC)
                                 .addReg(Ra)
-                .addOperand(MCOperand::createExpr(CallExpr));
-  Binary = getBinaryCodeForInstr(TmpInst, Fixups, STI);
+                                .addOperand(MCOperand::createExpr(CallExpr)),
+                            Fixups, STI);
   support::endian::write(OS, Binary, support::little);
 
   if (MI.getOpcode() == RISCV::PseudoTAIL)
     // Emit JALR X0, X6, 0
-    TmpInst = MCInstBuilder(RISCV::JALR).addReg(RISCV::X0).addReg(Ra).addImm(0);
+    Binary = getBinaryCodeForInstr(
+        MCInstBuilder(RISCV::JALR).addReg(RISCV::X0).addReg(Ra).addImm(0),
+        Fixups, STI);
   else
     // Emit JALR X1, X1, 0
-    TmpInst = MCInstBuilder(RISCV::JALR).addReg(Ra).addReg(Ra).addImm(0);
-  Binary = getBinaryCodeForInstr(TmpInst, Fixups, STI);
+    Binary = getBinaryCodeForInstr(
+        MCInstBuilder(RISCV::JALR).addReg(Ra).addReg(Ra).addImm(0), Fixups,
+        STI);
   support::endian::write(OS, Binary, support::little);
 }
 
 void RISCVMCCodeEmitter::encodeInstruction(const MCInst &MI, raw_ostream &OS,
                                            SmallVectorImpl<MCFixup> &Fixups,
                                            const MCSubtargetInfo &STI) const {
   const MCInstrDesc &Desc = MCII.get(MI.getOpcode());
   // Get byte count of instruction.