Re: [PATCH] x86/Intel: don't swap operands of MONITOR and MWAIT

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Mon, Jul 30, 2012 at 11:39 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 30.07.12 at 18:20, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>> On Tue, Jul 24, 2012 at 7:49 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>> Generally, the documentation doesn't allow for any explicit operands
>>> to be specified with MONITOR/MWAIT. To permit the more legible
>>> overriding of the address size via specifying operands, the option is
>>> being retained even in Intel mode, but operand swapping is being
>>> suppressed by this patch. This is both because it makes no sense here
>>> (all of the operands are inputs) and since not doing so would require
>>> a more intrusive fix to process_immext() (as it currently mis-numbers
>>> the operands in the diagnostic when in Intel mode).
>>>
>>> 2012-07-24  Jan Beulich <jbeulich@suse.com>
>>>
>>>         * config/tc-i386.c (md_assemble): Also suppress operand
>>>         swapping for MONITOR and MWAIT.
>>>
>>> --- a/gas/config/tc-i386.c
>>> +++ b/gas/config/tc-i386.c
>>> @@ -3100,6 +3100,8 @@ md_assemble (char *line)
>>>        && i.operands > 1
>>>        && (strcmp (mnemonic, "bound") != 0)
>>>        && (strcmp (mnemonic, "invlpga") != 0)
>>> +      && (strcmp (mnemonic, "monitor") != 0)
>>> +      && (strcmp (mnemonic, "mwait") != 0)
>>>        && !(operand_type_check (i.types[0], imm)
>>>            && operand_type_check (i.types[1], imm)))
>>>      swap_operands ();
>>>
>>
>> Will it break GCC:
>>
>> (define_insn "sse3_monitor"
>>   [(unspec_volatile [(match_operand:SI 0 "register_operand" "a")
>>                      (match_operand:SI 1 "register_operand" "c")
>>                      (match_operand:SI 2 "register_operand" "d")]
>>                     UNSPECV_MONITOR)]
>>   "TARGET_SSE3 && !TARGET_64BIT"
>>   "monitor\t%0, %1, %2"
>>   [(set_attr "length" "3")])
>
> Hmm, yes, quite obviously. Though imo it's a mistake for gcc to
> redundantly specify the operands in the first place.
>
> If this is indeed an issue, then I'll withdraw the patch, but would
> like to see the diagnostic fixed.
>

Please open a bug report and we can investigate what we
can do.

Thanks.

-- 
H.J.