[PATCH] D44928: [MC] Pass MCSubtargetInfo to fixupNeedsRelaxation and applyFixup

Peter Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 21 03:27:50 PDT 2018


peter.smith added a comment.

Thanks very much for the comments. Although the STI is available at the time the instruction is encoded; due to relaxation being a separate pass resolved after all instructions have been emitted we don't have the STI to pass to applyFixups() as all we have is a list of fragments. For example the MCRelaxableFragment has to store the STI as it might need to re-encode the instruction during relaxation. Thinking about some possible alternatives:

- Make another pass over the input during relaxation (and at other times we need the STI, such as emitting NOPS in alignment padding), so that we know what STI is in effect at the time. I think that this would be a much larger change, in effect making the assembler 2 pass.
- Maintain some kind of mapping table between fragment and STI. I think that this is logically equivalent to storing the STI in the fragment though.
- Store the STI per MCInst, this seems wasteful and I don't think it would work for writeNops() as these come from alignment and padding fragments.

In short I've not been able to come up with anything better. If I have missed something and there is a better alternative I'd be happy to try implementing it.



================
Comment at: lib/Target/ARM/MCTargetDesc/ARMAsmBackend.h:22-25
+  // The STI from the target triple the MCAsmBackend was instantiated with
+  // note that MCFragments may have a different local STI that should be
+  // used in preference.
   const MCSubtargetInfo &STI;
----------------
echristo wrote:
> I think we should just rely on a passed in STI wherever necessary and only store module/object level caches here. Also applies to isThumbMode below :)
> 
> Thoughts?
I've got some follow up patches that do this for STI; the other use is in writeNops() . Unfortunately the patches needed to do this are substantially larger and more complicated than this one. Given that writeNops() matters less for correctness than this one I thought it better to do in separate patches. I've not yet looked at isThumbMode but my intuition is that it should also be possible to do as a follow up patch.

Repeating the comment from earlier in the history:

I've added reviews for the support of writeNops(). My apologies for the size of the changes, the first three make sure the STI is recorded in the fragment so that we can retrieve it when calling writeNops. The final one passes STI through to writeNops and removes STI from the X86 and ARM AsmBackends.
1 [MC] Move bundling and MCSubtargetInfo to MCEncodedFragment [NFC] D45959
2 [MC] Add MCSubtargetInfo to MCPaddingFragment [NFC] D45960
3 [MC] Add MCSubtargetInfo to MCAlignFragment [NFC] D45961
4 [MC] Use local MCSubtargetInfo in writeNops D45962   


https://reviews.llvm.org/D44928





More information about the llvm-commits mailing list