Hi Bing, Yes I am planning to continue it after stage 1 opens.
Regards, Venkat. -----Original Message----- From: gcc-ow...@gcc.gnu.org [mailto:gcc-ow...@gcc.gnu.org] On Behalf Of Bin.Cheng Sent: Friday, April 03, 2015 7:16 AM To: Jeff Law Cc: gcc@gcc.gnu.org Subject: Re: Combine changes ASHIFT into mult for non-MEM rtx On Thu, Apr 2, 2015 at 8:32 PM, Jeff Law <l...@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
