This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
Re: Combine changes ASHIFT into mult for non-MEM rtx
- From: "Bin.Cheng" <amker dot cheng at gmail dot com>
- To: Jeff Law <law at redhat dot com>
- Cc: "gcc at gcc dot gnu dot org" <gcc at gcc dot gnu dot org>
- Date: Fri, 3 Apr 2015 09:45:42 +0800
- Subject: Re: Combine changes ASHIFT into mult for non-MEM rtx
- Authentication-results: sourceware.org; auth=none
- References: <CAHFci29EVWRou58MvePs16DQ59uJj=7aRR4QuNLRcSqDJrUJiA at mail dot gmail dot com> <551D36EE dot 6050507 at redhat dot com>
On Thu, Apr 2, 2015 at 8:32 PM, Jeff Law <law@redhat.com> wrote:
> On 04/02/2015 03:39 AM, Bin.Cheng wrote:
>>
>> Hi,
>> In function make_compound_operation, the code/comment says:
>>
>> case ASHIFT:
>> /* Convert shifts by constants into multiplications if inside
>> an address. */
>> if (in_code == MEM && CONST_INT_P (XEXP (x, 1))
>> && INTVAL (XEXP (x, 1)) < HOST_BITS_PER_WIDE_INT
>> && INTVAL (XEXP (x, 1)) >= 0
>> && SCALAR_INT_MODE_P (mode))
>> {
>>
>>
>> Right now, it changes ASHIFT in any SET into mult because of below code:
>>
>> /* Select the code to be used in recursive calls. Once we are inside
>> an
>> address, we stay there. If we have a comparison, set to COMPARE,
>> but once inside, go back to our default of SET. */
>>
>> next_code = (code == MEM ? MEM
>> : ((code == PLUS || code == MINUS)
>> && SCALAR_INT_MODE_P (mode)) ? MEM // <--------bogus?
>> : ((code == COMPARE || COMPARISON_P (x))
>> && XEXP (x, 1) == const0_rtx) ? COMPARE
>> : in_code == COMPARE ? SET : in_code);
>>
>> This seems an overlook to me. The effect is all targets have to
>> support the generated expression in the corresponding pattern. Some
>> times the generated expression is just too stupid and missed. For
>> example below insn is tried by combine:
>> (set (reg:SI 79 [ D.2709 ])
>> (plus:SI (subreg:SI (sign_extract:DI (mult:DI (reg:DI 1 x1 [ i ])
>> (const_int 2 [0x2]))
>> (const_int 17 [0x11])
>> (const_int 0 [0])) 0)
>> (reg:SI 0 x0 [ a ])))
>>
>>
>> It actually equals to
>> (set (reg/i:SI 0 x0)
>> (plus:SI (ashift:SI (sign_extend:SI (reg:HI 1 x1 [ i ]))
>> (const_int 1 [0x1]))
>> (reg:SI 0 x0 [ a ])))
>>
>> equals to below instruction on AARCH64:
>> add w0, w0, w1, sxth 1
>>
>>
>> Because of the existing comment, also because it will make backend
>> easier (I suppose), is it reasonable to fix this behavior in
>> combine.c? Another question is, if we are going to make the change,
>> how many targets might be affected?
>
> There's already a thread on this issue -- it's not a regression so I haven't
> pushed it forward and probably won't until stage1 reopens.
Indeed, Kugan pointed me to that thread. Glad somebody else is
looking after the problem.
Thanks,
bin
>
> jeff