[PATCH] Support APX PUSHP/POPP

[Cui, Lili]

> >>>> Then again, why is this constant needed for PUSHP/POPP, which have
> >>>> REX2.W set anyway, and hence that bit alone (when properly marked
> >>>> as
> >>>> consumed) should already allow to omit {rex2}. The question of
> >>>> adding a new constant (and how to suitably name it) would then be
> >>>> fully constrained to the JMPABS patch (for now, i.e. until such
> >>>> time that a 2nd insn appears which requires an otherwise empty REX2
> prefix).
> >>>>
> >>>
> >>> The current implementation is that no matter whether rex.w is
> >>> consumed or
> >> not, {rex2} will be printed as long as there is no Egpr. Of course,
> >> this place may be changed as discussed later.
> >>
> >> That's a bug then. If there is a REX2 prefix with just REX2.W set,
> >> and if that bit is properly consumed, no {rex2} should be printed
> >> imo. That's despite the same thing being expressable (in the common case;
> not here) with REX.W.
> >> Anything beyond that depends on the wider question of how to deal
> >> with _unused_ REX2 payload bits.
> >>
> >
> > For the last two instructions, although rex.w is consumed, we still need to
> print {rex2} to distinguish them.
> >
> > 0000000000000000 <_start>:
> >    0:   48 50                                 rex.W push %rax
> >    2:   d5 08 50                           pushp  %rax
> >    5:   48 6a 01                           rex.W push $0x1
> >    8:   d5 08 6a 01                     {rex2} push $0x1
> >    c:   48 c7 00 12 00 00 00                 movq   $0x12,(%rax)
> >   13:   d5 08 c7 00 12 00 00 00         {rex2} movq $0x12,(%rax)
> 
> Well, no, I don't really agree (especially not for the last one). But that goes
> back to the more general question on the printing of {rex2} without also
> indicating unconsumed bits.
> 

For the last one, rex2.w is consumed, but we still need to print {rex2} to distinguish it from rex, but now the problem is that we have different views on the specific information printed out, which comes back to the original question, Let’s wait for HJ for a few days. For the current patch I prefer to use REX2_SPECIAL, it needs this special handle.

> >>>>>>>> The new Rex2 attribute is not only wasteful (it can easily be a
> >>>>>>>> new enumerator used with OperandConstraint), but also
> misleading.
> >>>>>>>> We don't just need REX2 here, but we need it with REX2.W set.
> >>>>>>>> Even if from the tc-i386.c changes it looks as if that was
> >>>>>>>> happening implicitly (presumably due to the absence of
> >>>>>>>> NoRex64), naming still needs
> >>>>>> to properly reflect the purpose.
> >>>>>>>>
> >>>>>>>
> >>>>>>> You are right, rex2.w is set in process_suffix, since there is no
> NoRex64.
> >>>>>>>
> >>>>>>> How about Rex2W?
> >>>>>>> if (i.tm.opcode_modifier.rex2w)
> >>>>>>> {
> >>>>>>>     i.rex2_encoding = true;
> >>>>>>>     i.rex |= REX_W;   // add NoRex64 back, and set REX_W here.
> >>>>>>> }
> >>>>>>>
> >>>>>>>
> >>>>>>> Or just add a special judgment?
> >>>>>>>
> >>>>>>> if (t->mnem_off == MN_pushp || t->mnem_off == MN_popp) {
> >>>>>>>     i.rex2_encoding = true;
> >>>>>>>     i.rex |= REX_W;   // add NoRex64 back, and set REX_W here.
> >>>>>>> }
> >>>>>>
> >>>>>> I'd prefer the latter over a new attribute (albeit once again
> >>>>>> with the comment actually matching code), and perhaps I view the
> >>>>>> latter equal to my earlier suggestion.
> >>>>>>
> >>>>>
> >>>>> Ok, put them in process_operands, there's already a special
> >>>>> handler here to
> >>>> change the prefix.
> >>>>>
> >>>>> diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c index
> >>>>> 0dde2a9ad44..2f2b1b04d10 100644
> >>>>> --- a/gas/config/tc-i386.c
> >>>>> +++ b/gas/config/tc-i386.c
> >>>>> @@ -8715,6 +8715,13 @@ process_operands (void)
> >>>>>        i.tm.operands++;
> >>>>>      }
> >>>>>
> >>>>> +  /* PUSHP/POPP requires rex2.w == 1.  */  if (i.tm.mnem_off ==
> >>>>> + MN_pushp || i.tm.mnem_off == MN_popp)
> >>>>> +    {
> >>>>> +      i.rex2_encoding = true;
> >>>>> +      i.rex |= REX_W;
> >>>>> +    }
> >>>>
> >>>> Well, I'm sorry for not considering JMPABS earlier on, but with
> >>>> that also needing dealing with, I think I view my alternative
> >>>> suggestion as
> >> preferable.
> >>>> That'll scale better when also considering that down the road
> >>>> further such insns may appear. Whether it's actually
> >>>> OperandConstraint that we leverage here is secondary (it's not
> >>>> ideal because there's nothing
> >> operand related here).
> >>>> I'd be perfectly okay with some other attribute being suitably
> >>>> overloaded, whereas I continue to think that introducing new
> >>>> attributes should preferably be limited to either cases where more
> >>>> than just two or three templates use them or cases where otherwise
> >>>> it's impossible to avoid ambiguities. From earlier changes of mine
> >>>> the underlying reason ought to be pretty clear: Each new attribute
> >>>> consumes storage, and with thousands of templates growth of storage
> >>>> requirements should be balanced with how frequently an attribute is
> >>>> actually going to have a non-zero value. For example, with
> >>>> is_evex_encoding() gone a brief inspection suggests that it might
> >>>> be possible to overload Masking (or maybe Broadcast): They're
> >>>> applicable to EVEX templates only, and the class of insns we're
> >>>> discussing here is never going to be EVEX (or VEX). IOW not much
> >>>> different from the overloading of StaticRounding. Such an overload
> >>>> may then well be named Rex2 (as you had it, and considering its
> >>>> intended use also for
> >> JMPABS, plus taking into consideration that REX2.W will be set simply
> >> because of the absence of NoRex64).
> >>>>
> >>>
> >>> Haha, StaticRounding is really special, I tried "#define Rex2Req
> >>> Masking" and
> >> found that it will be used in i386-gen.c to identify EVEX
> >> (Broadcast...), then I tried VexW and SIB found that they are all
> >> used without precheck whether it was an vex instruction. Finally I
> >> wanted to re-use StaticRounding and found out that hulin already uses it
> for legacy insns.
> >>
> >> Hmm, I'm sorry for the trouble. I'm inclined to say OperandConstraint
> >> with a new #define it is then. Once everything's in I could then
> >> still see whether I can
> >> (reasonably) make e.g. Masking work here.
> >>
> >
> > Then I will create a new attribute Rex2 for it.
> 
> ???
> 

I can't use "#define Rex2Req Masking" instead of creating a new bitfile for Rex2Req, then I have to create a new bitfile for Rex2Req , or you want to use i.tm.mnem_off to handle it? both are ok to me.

Lili.