[PATCH] D45181: [RISCV] Add diff relocation support for RISC-V

[Alex Bradbury via Phabricator via llvm-commits]

asb added inline comments.


================
Comment at: lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp:42
+  bool requiresDiffExpressionRelocations() const override {
+    return STI.getFeatureBits()[RISCV::FeatureRelax];
+  }
----------------
simoncook wrote:
> I've been doing some additional thinking on this, and I don't know if having this depend on the relax feature bit might introduce a problem going into the future.
> 
> Looking at D45864, the way `.option rvc` and `.option norvc` are implemented is they add and remove feature bits. If in the future we support `.option norelax` through the same approach we can end up with a situation where we end up with correct DWARF depending on which of these two is used last.
> 
> If we want to do this unconditionally, then we need to track whether feature relax was ever enabled, possibly via a second feature bit that is set by `.option relax` but not cleared by `.option norelax` and have return true if either is set. Does this seem like a good approach?
I could see that argument. I could certainly see a counter-argument that says that if the user uses .option relax and .option norelax in the same section then they're doing something wrong and if things break they get to keep both pieces.

Maybe an alternative is to warn if relax/norelax is switched within the same section?


Repository:
  rL LLVM

https://reviews.llvm.org/D45181