[PATCHES] PR18303: Use appropriate Feature flags for encoding instructions

[Jim Grosbach]

On Jan 8, 2014, at 2:09 PM, David Woodhouse <dwmw2 at infradead.org> wrote:

> On Wed, 2014-01-08 at 13:34 -0800, Jim Grosbach wrote:
>> The current direction makes the simplifying assumption that what
>> actually changes is only the feature bits, so instead of passing
>> around whole new MCSubtargetInfo objects, we just keep a single one
>> and make it mutable. That’ll work out for now, but will break down
>> when we want to generalize. 
> 
> Well, not quite. Note that "the feature bits" as returned in a uint64_t
> by MCCodeEmitter::getAvailableFeatures() and subsequently passed to
> MCCodeEmitter::EncodeInstruction() are a private matter for the
> MCCodeEmitter in question. You might think that they match up with the
> MCSubtargetInfo bits; I couldn't possibly comment.
> 
> So the assumption is really "however the MCSubtargetInfo affects the
> code generation, that can be represented within 64 bits”.

Except that it’s not just code generation in the more general form of this problem. For example, we want to annotate assembly and disassembly with scheduling derived information, and that depends on the CPU and is part of what the MCSubtargetInfo contains (via a const MCSchedModel*).

> You could add a MCSubtargetInfo argument to the getAvailableFeatures()
> method, have the MCCodeEmitter extract the useful information it needs
> to affect its behaviour, and still not have to preserve a *full*
> MCSubtargetInfo object in the MCRelaxableFragment for later use as we
> were discussing on IRC.
> 
>> That said, it moves in the right direction and won’t actively
>> interfere with the more general solution later, which would just be
>> switching out the EmitterFeatures ivar with an MCSubtargetInfo& and
>> threading that through.
> 
> Right. It shouldn't be that hard to replace the uint64_t argument that
> I've threaded through it akk with a MCSubtargetInfo, if we really do
> need that later. And if we really do need the *full* object and not just
> a uint64_t representing the important information that the MCCodeEmitter
> in question needs.

*nod*

How hard would it be to do the real deal and remove the MCSubtargetInfo from the various target MCCodeEmitter implementations? We’ll be passing around the relevant info directly now anyway, so it shouldn’t be necessary. Theoretically. ;)

The main thing I want to avoid is having some of the interfaces maintain a state and some of them not. That’s just weird, confusing, and massively error prone for people hacking on the code later (i.e., I’m completely sure I’ll get it wrong and screw things up massively the next time I’m in there).

-Jim