This is the mail archive of the 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] - fix optimizations on vector ROTATERT in combine.c

>>>>> "Igor" == Igor Shevlyakov <> writes:

 > When combine.c (simplify_shift_const) converting ROTATERT to ROTATE new
 > shift count is calculated incorrectly for vector operands since each
 > element is rotated by count not whole vector as bit-stream. This patch
 > fixes it

Hmmm, I didn't know we had any ports that used rotate* on SIMD
operands.  And I don't think we've even defined what a rotate on SIMD
operands is supposed to do.

In any case, here are a few general comments related to style.

 > 2002-08-26  Igor Shevlyakov <>

 > 	* combine.c (simplify_shift_const): Converting from ROTATERT to
 > ROTATE for
 > 	  vector operands calculated shift count incorrectly.

ChangeLog entries should specify what the new behavior is, not what
was broken.  So something like this would be better:

        Calculate rotate count correctly for vector operands.

Please read the GNU Coding Standards at:

 >         /* Convert ROTATERT to ROTATE.  */
 > !       if (code == ROTATERT)
 > ! 	code = ROTATE, count = GET_MODE_BITSIZE (result_mode) - count;
 >         /* We need to determine what mode we will do the shift in.  If
 > the
 >   	 shift is a right shift or a ROTATE, we must always do it in the
 > mode
 > --- 9041,9053 ----
 >   	break;
 >         /* Convert ROTATERT to ROTATE.  */
 > !       if (code == ROTATERT) {

Curlies go on a separate line.

 > ! 	code = ROTATE;
 > ! 	if (VECTOR_MODE_P(result_mode))

Space after VECTOR_MODE_P.

 > ! 	  count = GET_MODE_BITSIZE (result_mode) /
 > GET_MODE_NUNITS(result_mode) - count;
 > ! 	else
 > !  	  count = GET_MODE_BITSIZE (result_mode)  - count;
 > !       }

You need a testcase.  You also need to specify how you tested this
(bootstrapped and regression tested on which platforms).

Otherwise, this looks reasonable, but a global maintainer will have to
approve this.


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