[PATCH] D135960: [RISCV] Allow LI with symbol difference as constant

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added a comment.

This seems rather limited in use...

Also nothing's testing the error case.



================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2729
       // Just convert to an addi. This allows compatibility with gas.
-      emitToStreamer(Out, MCInstBuilder(RISCV::ADDI)
-                              .addReg(Reg)
-                              .addReg(RISCV::X0)
-                              .addExpr(Op1.getExpr()));
+      MCInst Addi = MCInstBuilder(RISCV::ADDI)
+                        .addReg(Reg)
----------------
jrtc27 wrote:
> reames wrote:
> > reames wrote:
> > > Please separate this change as it's own diff so I can review it separately.  
> > It also looks like we have the same problem in emitAuipcInstPair used for a bunch of the other cases.
> This would probably be nicer if MCInstBuilder could take an SMLoc in its constructor and/or had a setLoc method. There are surely many cases of backends throwing this away as a result... though this is AsmParser so debug info is barely a thing.
Oh right SMLoc not SDLoc, so not debug info at least


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp:56
       {"fixup_riscv_lo12_i", 20, 12, 0},
+      {"fixup_riscv_12_i", 20, 12, 0},
       {"fixup_riscv_lo12_s", 0, 32, 0},
----------------
Hm, this is the only fixup that doesn't correspond to a relocation?


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp:430
+    }
+    return Value;
   case RISCV::fixup_riscv_lo12_s:
----------------
Don't you still need to mask this so the high bits of negative values get cleared?


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

https://reviews.llvm.org/D135960