[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