[PATCH] D77851: [X86][MC] Make -x86-pad-max-prefix-size compatible with --mc-relax-all
Fangrui Song via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 16 23:25:38 PDT 2020
MaskRay added a comment.
In D77851#1988163 <https://reviews.llvm.org/D77851#1988163>, @skan wrote:
> In D77851#1986486 <https://reviews.llvm.org/D77851#1986486>, @Razer6 wrote:
>
> > It seems that this change breaks the RISCV backend. RISCVAsmBackend::relaxInstruction <https://github.com/llvm/llvm-project/blob/69040d5b0bfa59edacc2ad10d517b4270bf76845/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp#L142> assumes that the `Relaxed` parameter is a fresh uninitialized `MCInst`. With this change, invalid instructions with too many operands are generated.
>
>
> We can fix this in two ways
>
> 1. Change the control flow in MCObjectStreamer.cpp
> 2. or create a fresh uninitialized `MCInst` at the beginning of `RISCVAsmBackend::relaxInstruction`, and after relaxation, assign its value to `Relaxed` at the end of `RISCVAsmBackend::relaxInstruction`.
>
> Which way do you think is better? Personally, I prefer the second one, since `getAssembler().getBackend().relaxInstruction(Relaxed, STI, Relaxed)` is called in a loop, the assumption that third argument of `relaxInstruction` is a fresh uninitialized `MCInst` is very strange.
Special casing RISC-V looks good to me.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77851/new/
https://reviews.llvm.org/D77851
More information about the llvm-commits
mailing list