[PATCH] x86: Add {load}/{store} tests for apx instructions.

Jan Beulich jbeulich@suse.com
Wed Jul 3 06:55:01 GMT 2024


On 03.07.2024 03:10, Cui, Lili wrote:
>> On 02.07.2024 15:49, Cui, Lili wrote:
>>>> On 02.07.2024 10:22, Jan Beulich wrote:
>>>>> On 02.07.2024 09:09, Cui, Lili wrote:
>>>>>>> On 01.07.2024 15:42, Cui, Lili wrote:
>>>>>>>> I met some issues when adding CTESTcc and CCMPcc.
>>>>>>>>
>>>>>>>> For example, the disassembler for "{load} ccmpbl %edx, %eax" now
>>>>>>>> looks a bit
>>>>>>> weird.
>>>>>>>>
>>>>>>>> "ccmpbl {dfv=}.s %edx,%eax"
>>>>>>>
>>>>>>> Well ... Something clearly needs doing about this. I'm now even
>>>>>>> more convinced we want tests for these.
>>>>>>>
>>>>>> Indeed, I added a macro %SW to indicate operands were swapped when
>>>> suffix_always is true.  Are you ok with it?
>>>>>
>>>>> In principle (and if then used consistently), why not. However, as
>>>>> long as you use it only for ...
>>>>>
>>>>>> --- a/opcodes/i386-dis-evex.h
>>>>>> +++ b/opcodes/i386-dis-evex.h
>>>>>> @@ -940,8 +940,8 @@ static const struct dis386 evex_table[][256] = {
>>>>>>      /* 38 */
>>>>>>      { "%NEccmp%SCB%DF",                { Eb, Gb }, 0 },
>>>>>>      { "%NEccmp%SCS%DF",                { Ev, Gv }, PREFIX_NP_OR_DATA },
>>>>>> -    { "%NEccmp%SCB%DF",                { Gb, EbS }, 0 },
>>>>>> -    { "%NEccmp%SCS%DF",                { Gv, EvS }, PREFIX_NP_OR_DATA },
>>>>>> +    { "%NEccmp%SCB%SW%DF",     { Gb, Eb }, 0 },
>>>>>> +    { "%NEccmp%SCS%SW%DF",     { Gv, Ev }, PREFIX_NP_OR_DATA },
>>>>>
>>>>> ... CCMPcc / CTESTcc, couldn't you make %DF fulfill this job as well?
>>>>
>>>> I'm sorry, this was half rubbish, I'm afraid. TEST (and hence
>>>> CTESTcc) has only a single encoding, and hence would never get .s
>>>> suffixes. So if there's no better way, a new %SW macro (just for
>>>> CCMPcc as you have it above) it is then. An alternative might be to
>>>> convert %DF to a DFV_Fixup() handler (to defer that output enough so
>>>> that normal .s printing would come first), yet that may be more overhead
>> than what we have right now for the {dfv=...} printing.
>>>>
>>>
>>> I'm trying to get the opcode from ins->codep and if the opcode > 39 and
>> Msuffix is ​​true, add a ".s" to it.  Do you like this version ?
>>
>> I'm afraid I don't, since I consider ...
>>
>>> -    { "%NEccmp%SCB%DF",                { Gb, EbS }, 0 },
>>> -    { "%NEccmp%SCS%DF",                { Gv, EvS }, PREFIX_NP_OR_DATA },
>>> +    { "%NEccmp%SCB%DF",                { Gb, Eb }, 0 },
>>> +    { "%NEccmp%SCS%DF",                { Gv, Ev }, PREFIX_NP_OR_DATA },
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>> --- a/opcodes/i386-dis.c
>>> +++ b/opcodes/i386-dis.c
>>> @@ -10587,6 +10589,11 @@ putop (instr_info *ins, const char
>> *in_template, int sizeflag)
>>>             }
>>>           else if (l == 1 && last[0] == 'D')
>>>             {
>>> +             if (sizeflag & SUFFIX_ALWAYS && (ins->codep[-1] == 0X3A
>>> + ||ins->codep[-1] == 0X3B))
>>
>> ... this extremely fragile. It was on more than one occasion that I thought it
>> might be helpful to know the major opcode while handling operands, but each
>> time I found another solution. First and foremost to avoid doing what you do
>> here. If we really want to allow access to the major opcode, it'll need properly
>> storing (along with encoding
>> space) in "ins".
>>
> 
> Yes, when I want to use major opcode for this case, I think it should be putted under ins, but I didn't find it, then I realized there might be no precedent for disassembler using major opcodes.
> 
>> As a minor remark (may save a comment on a later version), I also have
>> stylistic concerns with that line (beyond it apparently being too long). Please
>> can such be written as e.g.
>>
>>              if ((sizeflag & SUFFIX_ALWAYS) && (ins->codep[-1] | 1) == 0x3B))
>>
>> ?
> Sure, it is better, this usage is common in assembler.
> 
> Based on the above discussion, I think it is better to use %sw.

Right, that's what I said earlier. Down the road I may see about doing
this differently, but for now using %SW is going to be okay(ish). My
main dislike about it is how long (and hence increasingly difficult to
follow) "%NEccmp%SCB%SW%DF" is getting.

Jan


More information about the Binutils mailing list