[PATCH] D92795: [RISCV] Preserve Assembler Source Locations

[Sam Elliott via Phabricator via llvm-commits]

lenary created this revision.
lenary added reviewers: asb, luismarques, mundaym.
Herald added subscribers: frasercrmck, NickHung, evandro, apazos, sameer.abuasal, pzheng, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, hiraditya.
lenary requested review of this revision.
Herald added a subscriber: MaskRay.
Herald added a project: LLVM.

We weren't using the IDLoc available everywhere in the RISCVAsmParser. This
means we were losing potential source code location information.

This commit adds a way of providing the location (only once) to MCInstBuilder,
and uses that interface within the RISCVAsmParser, so location information isn't
lost.

I *think* (but I'm not 100% sure) this would manifest as a lack of location
information for e.g. `call` and `li` instructions when assembled into object
files.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92795

Files:
  llvm/include/llvm/MC/MCInstBuilder.h
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp


Index: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
===================================================================
--- llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -98,7 +98,8 @@
 
   // Helper to emit a combination of LUI, ADDI(W), and SLLI instructions that
   // synthesize the desired immedate value into the destination register.
-  void emitLoadImm(MCRegister DestReg, int64_t Value, MCStreamer &Out);
+  void emitLoadImm(MCRegister DestReg, int64_t Value, SMLoc IDLoc,
+                   MCStreamer &Out);
 
   // Helper to emit a combination of AUIPC and SecondOpcode. Used to implement
   // helpers such as emitLoadLocalAddress and emitLoadAddress.
@@ -2163,7 +2164,7 @@
   S.emitInstruction((Res ? CInst : Inst), getSTI());
 }
 
-void RISCVAsmParser::emitLoadImm(MCRegister DestReg, int64_t Value,
+void RISCVAsmParser::emitLoadImm(MCRegister DestReg, int64_t Value, SMLoc IDLoc,
                                  MCStreamer &Out) {
   RISCVMatInt::InstSeq Seq;
   RISCVMatInt::generateInstSeq(Value, isRV64(), Seq);
@@ -2172,11 +2173,13 @@
   for (RISCVMatInt::Inst &Inst : Seq) {
     if (Inst.Opc == RISCV::LUI) {
       emitToStreamer(
-          Out, MCInstBuilder(RISCV::LUI).addReg(DestReg).addImm(Inst.Imm));
+          Out,
+          MCInstBuilder(RISCV::LUI, IDLoc).addReg(DestReg).addImm(Inst.Imm));
     } else {
-      emitToStreamer(
-          Out, MCInstBuilder(Inst.Opc).addReg(DestReg).addReg(SrcReg).addImm(
-                   Inst.Imm));
+      emitToStreamer(Out, MCInstBuilder(Inst.Opc, IDLoc)
+                              .addReg(DestReg)
+                              .addReg(SrcReg)
+                              .addImm(Inst.Imm));
     }
 
     // Only the first instruction has X0 as its source.
@@ -2200,13 +2203,14 @@
 
   const RISCVMCExpr *SymbolHi = RISCVMCExpr::create(Symbol, VKHi, Ctx);
   emitToStreamer(
-      Out, MCInstBuilder(RISCV::AUIPC).addOperand(TmpReg).addExpr(SymbolHi));
+      Out,
+      MCInstBuilder(RISCV::AUIPC, IDLoc).addOperand(TmpReg).addExpr(SymbolHi));
 
   const MCExpr *RefToLinkTmpLabel =
       RISCVMCExpr::create(MCSymbolRefExpr::create(TmpLabel, Ctx),
                           RISCVMCExpr::VK_RISCV_PCREL_LO, Ctx);
 
-  emitToStreamer(Out, MCInstBuilder(SecondOpcode)
+  emitToStreamer(Out, MCInstBuilder(SecondOpcode, IDLoc)
                           .addOperand(DestReg)
                           .addOperand(TmpReg)
                           .addExpr(RefToLinkTmpLabel));
@@ -2379,7 +2383,7 @@
     if (Op1.isExpr()) {
       // We must have li reg, %lo(sym) or li reg, %pcrel_lo(sym) or similar.
       // Just convert to an addi. This allows compatibility with gas.
-      emitToStreamer(Out, MCInstBuilder(RISCV::ADDI)
+      emitToStreamer(Out, MCInstBuilder(RISCV::ADDI, IDLoc)
                               .addReg(Reg)
                               .addReg(RISCV::X0)
                               .addExpr(Op1.getExpr()));
@@ -2391,7 +2395,7 @@
     // represents the expected signed 64-bit number.
     if (!isRV64())
       Imm = SignExtend64<32>(Imm);
-    emitLoadImm(Reg, Imm, Out);
+    emitLoadImm(Reg, Imm, IDLoc, Out);
     return false;
   }
   case RISCV::PseudoLLA:
Index: llvm/include/llvm/MC/MCInstBuilder.h
===================================================================
--- llvm/include/llvm/MC/MCInstBuilder.h
+++ llvm/include/llvm/MC/MCInstBuilder.h
@@ -15,6 +15,7 @@
 #define LLVM_MC_MCINSTBUILDER_H
 
 #include "llvm/MC/MCInst.h"
+#include "llvm/Support/SMLoc.h"
 
 namespace llvm {
 
@@ -27,6 +28,11 @@
     Inst.setOpcode(Opcode);
   }
 
+  MCInstBuilder(unsigned Opcode, SMLoc Loc) {
+    Inst.setOpcode(Opcode);
+    Inst.setLoc(Loc);
+  }
+
   /// Add a new register operand.
   MCInstBuilder &addReg(unsigned Reg) {
     Inst.addOperand(MCOperand::createReg(Reg));


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D92795.310039.patch
Type: text/x-patch
Size: 3875 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201207/b60da5ef/attachment.bin>