Home Original page

[PATCH] x86: Extract extended states from instruction template

Jan Beulich jbeulich@suse.com
Fri Jul 10 15:17:44 GMT 2020 
On 10.07.2020 15:41, H.J. Lu wrote:
> On Fri, Jul 10, 2020 at 12:02 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 09.07.2020 18:22, H.J. Lu wrote:
>>> On Thu, Jul 9, 2020 at 9:17 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Thu, Jul 9, 2020 at 9:13 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>> On 09.07.2020 17:44, H.J. Lu wrote:
>>>>>> Should we add a field to i386 opcode table for this?
>>>>>
>>>>> I was wanting to try without, to see how much logic would be needed.
>>>>> If it gets too much, doing so may be the better choice.
>>>>
>>>> I am checking in this patch.
>>>
>>> Here is the right patch.
>>
>> Well, this is certainly an improvement. But it doesn't come close to
>> what's needed. Again - imo the concept of i.has_reg* isn't helpful
>> at all. It's not the registers an insn _actually_ accesses, but the
>> ones the template says it might access. Think of vfpclass* with a
>> memory operand, for example. Or see some of the extra checks of
>> certain opcodes when setting the MMX bit, which could be avoided by
>> using such an alternative model.
>>
>> I'm leaving aside here special cases where the templates don't carry
>> enough information. I'm also leaving aside the need to e.g. record
>> mask register use in some form (maybe translating to ZMM use if you
>> want to avoid introducing a separate tracking bit), as well as
>> various other issues I had spotted without looking very closely yet.
> 
> Here is a patch to extract extended states from operand types in
> instruction template and set xstate_zmm for master register move.

Ah yes, I appreciate this move.

>> I believe a fundamental issue here is the very scarce testing: While
>> it may be avoidable to add an attribute to the insn templates, there
>> should be full testing coverage of _all_ insns with _all_ possible
>> operand type combinations, if the resulting notes are to be
>> dependable. If such (presumably table based) testing was there (and
>> of course if the table reflected the real needs rather than what
>> gas currently happens to produce), you'd easily notice the issues
>> I've spotted (and likely more).
> 
> This is just a starting point.  I will fix any issues discovered.

I'll see to get to running this over the various little examples I
have, once it got committed (and I got around to re-basing).

I don't think altering existing test cases is a good idea though -
accumulating the flags from multiple instructions will leave it
unclear which insn caused which flag(s) to get set. As said before,
I am of the pretty strong opinion that testsuite-wise the only
helpful thing here is to individually test insns (and in various
cases even the same insn with varying operands).

Jan
More information about the Binutils mailing list