[PATCH] D77851: [X86][MC] Make -x86-pad-max-prefix-size compatible with --mc-relax-all

Kan Shengchen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 17 23:57:58 PDT 2020


skan marked an inline comment as done.
skan added inline comments.


================
Comment at: llvm/lib/MC/MCObjectStreamer.cpp:404
     while (getAssembler().getBackend().mayNeedRelaxation(Relaxed, STI))
       getAssembler().getBackend().relaxInstruction(Relaxed, STI, Relaxed);
     EmitInstToData(Relaxed, STI);
----------------
LuoYuanke wrote:
> If target expected "Relaxed" be a fresh MCInst, then this line also has problem. I think for RISC-V, this line isn't been executed, so the problem is not exposed before. I prefer to fix the bug in MCObjectStreamer.cpp, because some other target may also append new operand to Relaxed MCInst. And the relaxInstruction() API seems imply the Relaxed MCInst is fresh. 
> 
> ```
>     while (getAssembler().getBackend().mayNeedRelaxation(Inst, STI)) {
>       MCInst Relaxed;
>       getAssembler().getBackend().relaxInstruction(Inst, STI, Relaxed);
>       Inst = Relaxed;
>     }
> 
> ```
The code you paste doesn't make sense, `Inst` is a `const MCInst &`.


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