[PATCH] x86: Extract extended states from instruction template

[H.J. Lu]

On Fri, Jul 10, 2020 at 8:17 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> 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).

Fixed.

This is the updated patch I am checking in.

Thanks.

-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-x86-Extract-extended-states-from-instruction-templat.patch
Type: text/x-patch
Size: 9020 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20200710/e2a6f853/attachment-0001.bin>