Bug in binop rotate ?

Bug in binop rotate ?

[Andrew Hutchinson]

I have been adding rotate capability to AVR port and have come across 
what I think is bug in
optabs.c: expand_binop()

This occurs during a rotate expansion. For example

target  = op0  rotated by op1

In the particular situation (code extract below) it tries a reverse 
rotate of (bits - op1). Where this expression is expanded as a simple 
integer,
a negation or subtraction depending on type of op1 and target.

The expansion of the subtraction is using the mode of the target - I 
believe it should be using the mode of op1.
The mode of the rotation  amount need not be the same as the target.

target:DI = Op0:DI rotate op1:HI

In my testcase it is not and I get  asserts latter in simplfy_rtx.

The negation mode looks equally wrong.

Am I mistaken?


  /* If we were trying to rotate, and that didn't work, try rotating
     the other direction before falling back to shifts and bitwise-or.  */
  if (((binoptab == rotl_optab
    && optab_handler (rotr_optab, mode)->insn_code != CODE_FOR_nothing)
       || (binoptab == rotr_optab
       && optab_handler (rotl_optab, mode)->insn_code != CODE_FOR_nothing))
      && mclass == MODE_INT)
    {
      optab otheroptab = (binoptab == rotl_optab ? rotr_optab : rotl_optab);
      rtx newop1;
      unsigned int bits = GET_MODE_BITSIZE (mode);

      if (CONST_INT_P (op1))
    newop1 = GEN_INT (bits - INTVAL (op1));
      else if (targetm.shift_truncation_mask (mode) == bits - 1)
    newop1 = negate_rtx (mode, op1);
      else
    newop1 = expand_binop (mode, sub_optab,
                   GEN_INT (bits), op1,
                   NULL_RTX, unsignedp, OPTAB_DIRECT);

Re: Bug in binop rotate ?

[Richard Guenther]

On Sat, Oct 17, 2009 at 3:47 PM, Andrew Hutchinson
<andrewhutchinson@cox.net> wrote:
> I have been adding rotate capability to AVR port and have come across what I
> think is bug in
> optabs.c: expand_binop()
>
> This occurs during a rotate expansion. For example
>
> target  = op0  rotated by op1
>
> In the particular situation (code extract below) it tries a reverse rotate
> of (bits - op1). Where this expression is expanded as a simple integer,
> a negation or subtraction depending on type of op1 and target.
>
> The expansion of the subtraction is using the mode of the target - I believe
> it should be using the mode of op1.
> The mode of the rotation  amount need not be the same as the target.
>
> target:DI = Op0:DI rotate op1:HI
>
> In my testcase it is not and I get  asserts latter in simplfy_rtx.
>
> The negation mode looks equally wrong.
>
> Am I mistaken?

I think you are correct.

Richard.

>
>  /* If we were trying to rotate, and that didn't work, try rotating
>    the other direction before falling back to shifts and bitwise-or.  */
>  if (((binoptab == rotl_optab
>   && optab_handler (rotr_optab, mode)->insn_code != CODE_FOR_nothing)
>      || (binoptab == rotr_optab
>      && optab_handler (rotl_optab, mode)->insn_code != CODE_FOR_nothing))
>     && mclass == MODE_INT)
>   {
>     optab otheroptab = (binoptab == rotl_optab ? rotr_optab : rotl_optab);
>     rtx newop1;
>     unsigned int bits = GET_MODE_BITSIZE (mode);
>
>     if (CONST_INT_P (op1))
>   newop1 = GEN_INT (bits - INTVAL (op1));
>     else if (targetm.shift_truncation_mask (mode) == bits - 1)
>   newop1 = negate_rtx (mode, op1);
>     else
>   newop1 = expand_binop (mode, sub_optab,
>                  GEN_INT (bits), op1,
>                  NULL_RTX, unsignedp, OPTAB_DIRECT);
>

Re: Bug in binop rotate ?

[Andrew Hutchinson]

Thanks for your review.

I have submitted bug report.



Richard Guenther wrote:
> On Sat, Oct 17, 2009 at 3:47 PM, Andrew Hutchinson
> <andrewhutchinson@cox.net> wrote:
>   
>> I have been adding rotate capability to AVR port and have come across what I
>> think is bug in
>> optabs.c: expand_binop()
>>
>> This occurs during a rotate expansion. For example
>>
>> target  = op0  rotated by op1
>>
>> In the particular situation (code extract below) it tries a reverse rotate
>> of (bits - op1). Where this expression is expanded as a simple integer,
>> a negation or subtraction depending on type of op1 and target.
>>
>> The expansion of the subtraction is using the mode of the target - I believe
>> it should be using the mode of op1.
>> The mode of the rotation  amount need not be the same as the target.
>>
>> target:DI = Op0:DI rotate op1:HI
>>
>> In my testcase it is not and I get  asserts latter in simplfy_rtx.
>>
>> The negation mode looks equally wrong.
>>
>> Am I mistaken?
>>     
>
> I think you are correct.
>
> Richard.
>
>   
>>  /* If we were trying to rotate, and that didn't work, try rotating
>>    the other direction before falling back to shifts and bitwise-or.  */
>>  if (((binoptab == rotl_optab
>>   && optab_handler (rotr_optab, mode)->insn_code != CODE_FOR_nothing)
>>      || (binoptab == rotr_optab
>>      && optab_handler (rotl_optab, mode)->insn_code != CODE_FOR_nothing))
>>     && mclass == MODE_INT)
>>   {
>>     optab otheroptab = (binoptab == rotl_optab ? rotr_optab : rotl_optab);
>>     rtx newop1;
>>     unsigned int bits = GET_MODE_BITSIZE (mode);
>>
>>     if (CONST_INT_P (op1))
>>   newop1 = GEN_INT (bits - INTVAL (op1));
>>     else if (targetm.shift_truncation_mask (mode) == bits - 1)
>>   newop1 = negate_rtx (mode, op1);
>>     else
>>   newop1 = expand_binop (mode, sub_optab,
>>                  GEN_INT (bits), op1,
>>                  NULL_RTX, unsignedp, OPTAB_DIRECT);
>>
>>     
>
>