This is the mail archive of the gcc@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: Combine changes ASHIFT into mult for non-MEM rtx


On Thu, Apr 2, 2015 at 5:49 PM, Kugan <kugan.vivekanandarajah@linaro.org> wrote:
> On 02/04/15 20:39, 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?
>>
>
> I think https://gcc.gnu.org/ml/gcc-patches/2015-01/msg01020.html is
> related to this.

Hi Kugan,
Thanks for pointing me to this.  Actually it's exactly the same case I
am looking.
Glad to know that somebody else is looking after it, only that message
isn't followed up recently :(

Thanks,
bin

>
>
> Thanks,
> kugan


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