Re: [PATCH 1/9] x86: refine TPAUSE and UMWAIT

[Jan Beulich <jbeulich at suse dot com>]

On 05.03.2020 15:53, H.J. Lu wrote:
> On Thu, Mar 5, 2020 at 6:51 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 05.03.2020 15:37, H.J. Lu wrote:
>>> On Thu, Mar 5, 2020 at 6:08 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 05.03.2020 15:04, H.J. Lu wrote:
>>>>> On Thu, Mar 5, 2020 at 12:08 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>
>>>>>> On 04.03.2020 12:44, H.J. Lu wrote:
>>>>>>> On Wed, Mar 4, 2020 at 3:40 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>>> On 04.03.2020 12:36, H.J. Lu wrote:
>>>>>>>>> On Wed, Mar 4, 2020 at 1:37 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>>>>> Allowing 64-bit registers is misleading here: Elsewhere these get allowed
>>>>>>>>>> when there's no difference between either variant, because of 32-bit
>>>>>>>>>> destination registers having their upper halves zeroed in 64-bit mode.
>>>>>>>>>> Here, however, they're source registers, and hence specifying 64-bit
>>>>>>>>>> registers would lead to the ambiguity of whether the upper 32 bits
>>>>>>>>>> actually matter.
>>>>>>>>>>
>>>>>>>>>> Additionally, for proper code generation in 16-bit mode, IgnoreSize is
>>>>>>>>>> needed on both.
>>>>>>>>>
>>>>>>>>> Are there testcases to show IgnoreSize is needed on them?
>>>>>>>>
>>>>>>>> The situation with 16-bit test cases is rather poor anyway. I didn't
>>>>>>>> consider it reasonable to add such very special ones when far more
>>>>>>>> general ones don't exist. But if your question is to mean you demand
>>>>>>>
>>>>>>> Let's start from somewhere.
>>>>>>>
>>>>>>>> such to be added, then I'll (somewhat hesitantly) add/extend some.
>>>>>>>> Please clarify.
>>>>>>>
>>>>>>> Please add testcases.
>>>>>>
>>>>>> Actually they were there, in patch 2. I've moved them to this patch
>>>>>> and have just sent v1.1 for just this one patch.
>>>>>
>>>>> Do we need to adjust disassembler for 16-bit mode?
>>>>
>>>> I haven't checked, but (a) this would be an independent change
>>>> and (b) it would likely affect more than just the insns here
>>>> (see e.g. other parts of the series).
>>>
>>> Disassembler should be adjusted when IgnoreSize is added
>>> to tpause and umwait.
>>
>> But IgnoreSize has nothing to do with the disassembler. Right
>> now all I'm after is getting the assembler to produce correct
>> code. There are test cases to verify this is the case. Whether
>> the disassembler deals with any of this correctly (including
>> also the insns getting IgnoreSize added by "x86: add missing
>> IgnoreSize", which you've already approved) is an entirely
>> separate topic, which I may be willing to look into down the
>> road, but not now and here.
>>
>> More generally, when anyone fixes one bug, why should they get
>> penalized to have their bug fix accepted only when they take
>> the - perhaps significant amount of - time to also fix another,
>> unrelated bug? In the case here, it is definitely not my fault
>> if 16-bit handling is in a bad state. And I can't afford fixing
>> all issues there are in one go.
> 
> Then don't add IgnoreSize to tpause and umwait for now.

You're kidding. Once again - where is the connection between
adding IgnoreSize and disassembler behavior? I want code to
be generated correctly, no more and no less.

Jan