[PATCH] D153097: [RISCV] Make linker-relaxable instructions terminate MCDataFragment

[Sergei Barannikov via Phabricator via llvm-commits]

barannikov88 added a comment.

`MCObjectStreamer::emitInstructionImpl` has some logic deciding whether the instruction can be appended to a data fragment or should go in its own fragment.
I would expect the check to be there instead of `MCELFStreamer::emitInstToData`.
For this to be possible `encodeInstruction` should be pulled out too, so that you can examine Fixups.
As I can see, all implementations of `emitInstrToData` call `encodeInstruction`. (Except MCDXContainerStreamer,
but I guess this method was overridden just for the vtable to be pinned to cpp file.)



================
Comment at: llvm/lib/MC/MCELFStreamer.cpp:634
+    DF->setLinkerRelaxable();
   DF->getContents().append(Code.begin(), Code.end());
 
----------------
MaskRay wrote:
> barannikov88 wrote:
> > * Maybe add a new fixup flag to `MCFixupKindInfo.h` and check if it is set?
> > * Can Fixups contain more than one element? If it can, why are you checking only the last one?
> > 
> Linker-relaxable instructions have two relocations and the second (last) one is `R_RISCV_RELAX`. There is just one relocation type, so adding a `FixupKindFlags` flag appears to be overkill. In addition, calling `getFixupKindInfo` adds some overhead to non-RISCV targets.
> 
> The current way causes the least overhead to non-RISCV targets.
> 
There is not much of overhead, but makes the solution scalable and avoids target-specific changes to common code.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153097