This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


>>> 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.

Jan


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]