This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Remove ix86_legitimate_combined_insn
- From: Uros Bizjak <ubizjak at gmail dot com>
- To: "H.J. Lu" <hjl dot tools at gmail dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 19 Apr 2016 17:59:01 +0200
- Subject: Re: [PATCH] Remove ix86_legitimate_combined_insn
- Authentication-results: sourceware.org; auth=none
- References: <20160419144931 dot GB7801 at intel dot com> <CAFULd4b10NVsgdRq9j5VGcbYGY2VPgG7dU3sC8Qwtrx9+z1KPQ at mail dot gmail dot com> <CAMe9rOpEif5ZahcR9D916YGj_ZSMM02gQYqxNDPCjj_GHH3bwQ at mail dot gmail dot com> <CAFULd4ak_w4W0yq8ugdQOi6iw+DptLcagRj=TXTgjxeMzUspyg at mail dot gmail dot com> <CAMe9rOq8+rMS60mOc8Xr8Lvwr3KemPEaWwBceYRkn=SRO0pChQ at mail dot gmail dot com>
On Tue, Apr 19, 2016 at 5:36 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Apr 19, 2016 at 8:27 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> On Tue, Apr 19, 2016 at 5:18 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Tue, Apr 19, 2016 at 8:08 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>>> On Tue, Apr 19, 2016 at 4:49 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>>>>>
>>>>> From INSTRUCTION EXCEPTION SPECIFICATION section in Intel software
>>>>> developer manual volume 2, only legacy SSE instructions with memory
>>>>> operand not 16-byte aligned get General Protection fault. There is
>>>>> no need to check 1, 2, 4, 8 byte alignments. Since x86 backend has
>>>>> accurate constraints and predicates for 16-byte alignment, we can
>>>>> remove ix86_legitimate_combined_insn.
>>>>>
>>>>> Tested on x86-64. OK for trunk?
>>>>
>>>> No. This function also handles cases where invalid hard register gets
>>>> propagated into the insn during the combine pass, leading to spill
>>>> failure later.
>>>>
>>>
>>> ix86_legitimate_combined_insn was added to work around the
>>> reload issue:
>>
>> Sorry, I'm not convinced. Please see [1].
>>
>> You should remove only this part, together with now unused ssememalign
>> attribute.
>>
>> - /* For pre-AVX disallow unaligned loads/stores where the
>> - instructions don't support it. */
>> - if (!TARGET_AVX
>> - && VECTOR_MODE_P (mode)
>> - && misaligned_operand (op, mode))
>> - {
>> - unsigned int min_align = get_attr_ssememalign (insn);
>> - if (min_align == 0
>> - || MEM_ALIGN (op) < min_align)
>> - return false;
>> - }
>>
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46829
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46843
>>
>>> LRA doesn't have those limitation. Removing
>>> ix86_legitimate_combined_insn causes no regressions.
>>
>> [1] https://gcc.gnu.org/ml/gcc-patches/2012-08/msg01195.html
>>
>> Uros.
>
> Here is the updated patch. OK for trunk if there is no regression
> on x86-64?
OK...
BTW: I really hope that "INSTRUCTION EXCEPTION SPECIFICATION section"
quoted above is correct - we will quickly find out.
Thanks,
Uros.