Re: [PATCH 3/5] x86: Add -mindirect-branch-register

[Uros Bizjak <ubizjak at gmail dot com>]

On Mon, Jan 15, 2018 at 7:11 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Jan 15, 2018 at 8:54 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> On Mon, Jan 15, 2018 at 4:05 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Sun, Jan 14, 2018 at 1:23 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Sun, Jan 14, 2018 at 10:52 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>>>> On Sun, Jan 14, 2018 at 7:08 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>> On Sun, Jan 14, 2018 at 9:51 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>>>>>> -  (ior (and (not (match_test "TARGET_X32"))
>>>>>>> +  (ior (and (not (match_test "TARGET_X32
>>>>>>> +      || ix86_indirect_branch_thunk_register"))
>>>>>>>      (match_operand 0 "sibcall_memory_operand"))
>>>>>>> -       (and (match_test "TARGET_X32 && Pmode == DImode")
>>>>>>> +       (and (match_test "TARGET_X32 && Pmode == DImode
>>>>>>> + && !ix86_indirect_branch_thunk_register")
>>>>>>>      (match_operand 0 "GOT_memory_operand"))))
>>>>>>>
>>>>>>> Is this patch just trying to disable the predicate when
>>>>>>> ix86_indirect_branch_thunk_register is set? Because this is what this
>>>>>>> convoluted logic does.
>>>>>>
>>>>>> Yes, we want to disable all indirect branch via memory with
>>>>>> -mindirect-branch-register, just like -mx32.   We could do
>>>>>>
>>>>>> #idefine TARGET_INDIRECT_BRANCH_REGISTER \
>>>>>>  (TARGER_X32 ||  ix86_indirect_branch_thunk_register)
>>>>>
>>>>> Index: predicates.md
>>>>> ===================================================================
>>>>> --- predicates.md       (revision 256666)
>>>>> +++ predicates.md       (working copy)
>>>>> @@ -710,11 +710,10 @@
>>>>>    (ior (match_test "constant_call_address_operand
>>>>>                      (op, mode == VOIDmode ? mode : Pmode)")
>>>>>         (match_operand 0 "call_register_no_elim_operand")
>>>>> -       (ior (and (not (match_test "TARGET_X32
>>>>> -                                  || ix86_indirect_branch_thunk_register"))
>>>>> +       (and (not (match_test "ix86_indirect_branch_thunk_register"))
>>>>> +           (ior (and (not (match_test "TARGET_X32")))
>>>>>                  (match_operand 0 "memory_operand"))
>>>>> -           (and (match_test "TARGET_X32 && Pmode == DImode
>>>>> -                             && !ix86_indirect_branch_thunk_register")
>>>>> +                (and (match_test "TARGET_X32 && Pmode == DImode")
>>>>>                  (match_operand 0 "GOT_memory_operand")))))
>>>>>
>>>>> or something like that.
>>>>>
>>>>
>>>> I am testing this patch.  OK for trunk if there is no regression?
>>>>
>>>
>>> Here is the updated patch.  Tested on i686 and x86-64.  OK for
>>> trunk?
>>
>> There are two of the same issues in constraints.md
>>
>> On further inspection, there are several new
>> ix86_indirect_branch_thunk_register conditions sprinkled around
>> predicates.md. The one in indirect_branch_operand is understandable
>> (but should be written as:
>>
>>   (ior (match_operand 0 "register_operand")
>>        (and (not (match_test "ix86_indirect_thunk_register"))
>>             (not (match_test "TARGET_X32"))
>>         (match_operand 0 "memory_operand"))))
>> )
>>
>> but the ones in GOT_memory_operand and GOT32_symbol_operand should
>> *not* be there, since these are simple pattern matches. Now we have
>> situation where e.g. call_got_x32 and sibcall_got_32 patterns never
>> match, and should be disabled with
>> ix86_indirect_branch_thunk_register. Please move
>> ix86_indirect_branch_thunk_register conditions out of these two
>> predicates.
>
> I break them into 4 smaller patches:
>
> https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01361.html
> https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01360.html
> https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01362.html
> https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01359.html

Hopefully, OK.

Thanks,
Uros.