[PATCH] x86-64: don't allow use of %axl as accumulator

H.J. Lu hjl.tools@gmail.com
Mon Nov 13 15:42:00 GMT 2017


On Mon, Nov 13, 2017 at 6:33 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 13.11.17 at 14:40, <hjl.tools@gmail.com> wrote:
>> On Mon, Nov 13, 2017 at 2:03 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 10.11.17 at 14:27, <hjl.tools@gmail.com> wrote:
>>>> On Fri, Nov 10, 2017 at 4:50 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> Just like %cxl can't be used as shift count register. Otherwise for
>>>>> consistency %cxl would need to gain "ShiftCount" and use of both ought
>>>>> to properly cause REX prefixes to be emitted.
>>>>
>>>> There are no good testcases for
>>>>
>>>> axl, Reg8|Acc|Byte, RegRex64, 0, Dw2Inval, Dw2Inval
>>>> cxl, Reg8, RegRex64, 1, Dw2Inval, Dw2Inval
>>>> dxl, Reg8, RegRex64, 2, Dw2Inval, Dw2Inval
>>>> bxl, Reg8, RegRex64, 3, Dw2Inval, Dw2Inval
>>>> bpl, Reg8, RegRex64, 5, Dw2Inval, Dw2Inval
>>>
>>> Not sure I understand: I'm only changing %axl - why would I need
>>> to introduce tests for anything else?
>>>
>>>> I am not sure if the whole thing is handled correctly.  If we want to change
>>>> them, we should make sure that they work correctly.
>>>
>>> "They" being exactly what (and again in the context of what this
>>> patch does)? I'm not overly happy to introduce new unrelated
>>> tests, but I may be looking into adding some if I clearly knew
>>> what you're after. But even then I would submit that as a
>>> separate change, so you'd have to make up your mind on the
>>> change here independent of those anyway.
>>>
>>
>> There are fake registers:
>>
>> axl, Reg8|Acc|Byte, RegRex64, 0, Dw2Inval, Dw2Inval
>> cxl, Reg8, RegRex64, 1, Dw2Inval, Dw2Inval
>> dxl, Reg8, RegRex64, 2, Dw2Inval, Dw2Inval
>> bxl, Reg8, RegRex64, 3, Dw2Inval, Dw2Inval
>> spl, Reg8, RegRex64, 4, Dw2Inval, Dw2Inval
>> bpl, Reg8, RegRex64, 5, Dw2Inval, Dw2Inval
>> sil, Reg8, RegRex64, 6, Dw2Inval, Dw2Inval
>> dil, Reg8, RegRex64, 7, Dw2Inval, Dw2Inval
>>
>> There should be at least one testcase to show they work
>> correctly.
>
> Oh, are you suspecting axl, cxl, dxl, bxl, and bpl may not work
> correctly despite spl, sil, and dil being tested to do so? That's
> rather odd imo. I don't think we consistently test each and
> every other register out of whatever group to work right. And
> as said - while adding tests for them wouldn't be much work, it's
> orthogonal to what the patch here does, so I don't see why
> you make that other test (which has always been missing) a
> prereq for the one here.
>

I am not saying we need to test dxl for all instructions.  I
am only asking for ONE test.   Since we are changing this
piece of code, we should add at least ONE test for each
register.


-- 
H.J.



More information about the Binutils mailing list