This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC 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 PR69052]Set higher precedence for CONST_WIDE_INT than CONST_INT when canonicalizing addr expr


On Mon, Mar 7, 2016 at 10:27 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
> On Fri, Mar 4, 2016 at 6:06 PM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>>
>> On March 4, 2016 5:35:13 PM GMT+01:00, "Bin.Cheng" <amker.cheng@gmail.com> wrote:
>> >On Fri, Mar 4, 2016 at 11:57 AM, Richard Biener
>> ><richard.guenther@gmail.com> wrote:
>> >> On Fri, Mar 4, 2016 at 12:07 PM, Bin Cheng <Bin.Cheng@arm.com> wrote:
>> >>> Hi,
>> >>> A address canonicalization interface was introduced by my original
>> >patch to PR69052.  The interface sorts sub-parts in an address
>> >expression based on precedences defined by function
>> >commutative_operand_precedence.  It also assumes that all CONST_INT
>> >sub-parts are at the end of vector after sorting.  But this is not
>> >always true because commutative_operand_precedence sets the same
>> >precedence to both CONST_INT and CONST_WIDE_INT.  The patch could be
>> >broken in cases which have CONST_WIDE_INT sorted after CONST_INT.  Even
>> >though I don't think we can really run into such complicated case, I
>> >worked out a patch forcing CONST_INT to lower precedence than
>> >CONST_WIDE_INT, so that for sure it will be sorted after all other
>> >kinds sub-parts.
>> >>>
>> >>> This is an obvious change.  Bootstrap&test on x86_64, bootstrap&test
>> >on AArch64.  Is it OK for this stage?
>> >>
>> >> I believe the obvious change would be to change
>> >> commutative_operand_precedence to pre-CONST_WIDE_INT behavior, namely
>> >> giving CONST_WIDE_INT the same precedence as CONST_DOUBLE.
>> >Yes, but I am not sure what else this change will affect, so I made
>> >the smallest change in the original code.
>>
>> I don't like going with this kind of weirdo changes out of caution.  If you think it's too dangerous the push it back to gcc 7 and consider backporting for 6.2
>
> Hi,
> Attachment is the patch setting precedence of CONST_WIDE_INT to the
> same value as CONST_DOUBLE.  Bootstrap and test on x86_64 and AArch64,
> no regressions.

Ok.

Richard.

> Thanks,
> bin
>
> 2016-03-03  Bin Cheng  <bin.cheng@arm.com>
>
>     PR rtl-optimization/69052
>     * rtlanal.c (commutative_operand_precedence): Set higher precedence
>     to CONST_WIDE_INT.


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