[PATCH] D150004: [RISCV][MC] .debug_line/.debug_frame/.eh_frame: emit relocations for assembly input files with relaxation

Fri May 12 14:37:14 PDT 2023

MaskRay added a comment.

In D150004#4338878 <https://reviews.llvm.org/D150004#4338878>, @barannikov88 wrote:

>> Assembly source files typically don't write CFI directives, and a bug is mostly unnoticeable anyway.
>
> They do, if you use -save-temps :) But I agree that it is much less common.

Yes, `-save-temps` is like `clang -S a.c && clang -c a.s` with more intermediate steps, uncommon in projects' build systems.

> There is also `MCObjectStreamer::emitValueImpl`, which potentially has the same issue.
> I guess this is one of the reasons it was overridden for RISC-V, and fixing the base
> version would impose much more significant slowdowns for other targets.

The fast path of `MCObjectStreamer::emitValueImpl` does a simple operation, so keeping the fast path is more justified.
In addition, it has a `value evaluated as 300 is out of range` assembler error which is not implemented in the slow path yet.

On the contrary, the fast paths removed by this patch do fairly complex things, and I am concerned keeping them around.

I made a comment to D61584 <https://reviews.llvm.org/D61584> which introduced `canFold` in `MCExpr.cpp`.

> I'm wondering if RISCVELFStreamer::requiresFixups used in emitValueImpl could be
> promoted to a virtual function in MCAsmBackend or MCObjectStreamer and used
> in evaluateAsRelocatable? Or am I talking nonsense?

Making `RISCVELFStreamer::requiresFixups` available for MCAsmBackend makes sense.
I wish that we try improving its robustness of `RISCVELFStreamer::requiresFixups` first.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150004