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

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

Ping. Is anyone willing to approve or has any thoughts on a better design?

If I can summarise so far:

- Each MachineFunction can have its own Subtarget.
- On targets such as Arm, and I think also Mips, it is possible to change the Subtarget on the fly in assembler with a .arch directive.
- On Arm, relaxation and fixup calculation decisions can be affected by the Subtarget. PR36542 shows a real example with LTO where a default CPU of ARM7TDMI (Architecture v4t) was passed to MC, yet the MachineFunctions were generated using a much more recent Armv7A, this caused a fixup error as the code-generator was expecting the 2-byte Thumb1 Branch to be relaxed to a Thumb2 4-byte branch by MC.
- A legitimate use case of .arch is to do runtime selection of architecture so it is not appropriate to just pick either the lowest or highest Subtarget that we find in the Module.
- At the time we do relaxation and fixups (after layout) we don't know what the current active Subtarget is so we must have a way of recovering the Subtarget for relaxations and fixups.
- This patch extends the example of MCRelaxableFragment and stores the Subtarget in the MCDataFragment.
- There is at most one Subtarget per MCDataFragment so we create a new MCDataFragment when the Subtarget changes. This causes some complications with the AlignedBundling and RelaxAll used by NativeClient as all the instructions in a Bundle must be within the same fragment. This patch gives an error message if the Subtarget is changed within a bundle (possible in assembly only). A possible more complex alternative is to accept multiple Subtargets per Fragment but given the restrictions on target of NativeClient (ARM v7A instructions only) and deprecation of NativeClient it is extremely unlikely that this case will need to be handled.
- On both x86 and Arm the writeNops() function is also affected but is not handled by this patch due to the size of the change. There are a series of follow up patches (see earlier comments) for that.

At this stage I can't think of a logical alternative to making a mapping between Fragment and Subtarget. At present the Subtarget is stored in the fragment itself, it could be possible to maintain an external mapping table but I'm not sure whether that would be an improvement as it would need to be threaded through to where it is needed.


https://reviews.llvm.org/D44928