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 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.

jeff


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