[2/2] Enable elimination of zext/sext

On Wed, Aug 27, 2014 at 12:07 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
>> 2014-08-07  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>>
>>> * calls.c (precompute_arguments): Check
>>> promoted_for_signed_and_unsigned_p and set the promoted mode.
>>> (promoted_for_signed_and_unsigned_p): New function.
>>> (expand_expr_real_1): Check promoted_for_signed_and_unsigned_p
>>> and set the promoted mode.
>>> * expr.h (promoted_for_signed_and_unsigned_p): New function definition.
>>> * cfgexpand.c (expand_gimple_stmt_1): Call emit_move_insn if
>>> SUBREG is promoted with SRP_SIGNED_AND_UNSIGNED.
>>
>> This patch regresses:
>>
>> Running target unix
>> FAIL: libgomp.fortran/simd7.f90   -O2  execution test
>> FAIL: libgomp.fortran/simd7.f90   -Os  execution test
>>
>> on alphaev6-linux-gnu.
>>
>> So, the code assumes that it is possible to copy (reg:DI 540) directly
>> to (reg:DI 154). However, this is not the case, since we still have
>> garbage in the top 32bits.
>>
>> Reverting the part above fixes the runtime failure, since (insn 599) is now:
>>
>> (insn 599 598 0 (set (reg:DI 145 [ D.1694 ])
>>         (zero_extend:DI (subreg:SI (reg:DI 540) 0))) -1
>>      (nil))
>>
>> It looks to me that we have also to check the temp with SUBREG_PROMOTED_*.
>
> Yeah, that makes sense.

Something like following (untested) patch that also fixes the testcase perhaps?

-- cut here--
                  emit_move_insn (SUBREG_REG (target), SUBREG_REG (temp));
-- cut here

Uros.

On Wed, Aug 27, 2014 at 12:25:14PM +0200, Uros Bizjak wrote:
> Something like following (untested) patch that also fixes the testcase perhaps?
> 
> -- cut here--
> Index: cfgexpand.c
> ===================================================================
> --- cfgexpand.c (revision 214445)
> +++ cfgexpand.c (working copy)
> @@ -3322,6 +3322,7 @@ expand_gimple_stmt_1 (gimple stmt)
> 
>                 if ((SUBREG_PROMOTED_GET (target) == SRP_SIGNED_AND_UNSIGNED)
>                     && (GET_CODE (temp) == SUBREG)
> +                   && SUBREG_PROMOTED_VAR_P (temp)
>                     && (GET_MODE (target) == GET_MODE (temp))
>                     && (GET_MODE (SUBREG_REG (target)) == GET_MODE
> (SUBREG_REG (temp))))

Looks like a wrong order of the predicates in any case, first you should
check if it is a SUBREG, then SUBREG_PROMOTED_VAR_P and only then
SUBREG_PROMOTED_GET.  Also, the extra ()s around single line conditions
are unnecessary.

>                   emit_move_insn (SUBREG_REG (target), SUBREG_REG (temp));
> -- cut here
> 
> Uros.

	Jakub

On Mon, Sep 1, 2014 at 10:47 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Aug 27, 2014 at 12:25:14PM +0200, Uros Bizjak wrote:
>> Something like following (untested) patch that also fixes the testcase perhaps?
>>
>> -- cut here--
>> Index: cfgexpand.c
>> ===================================================================
>> --- cfgexpand.c (revision 214445)
>> +++ cfgexpand.c (working copy)
>> @@ -3322,6 +3322,7 @@ expand_gimple_stmt_1 (gimple stmt)
>>
>>                 if ((SUBREG_PROMOTED_GET (target) == SRP_SIGNED_AND_UNSIGNED)
>>                     && (GET_CODE (temp) == SUBREG)
>> +                   && SUBREG_PROMOTED_VAR_P (temp)
>>                     && (GET_MODE (target) == GET_MODE (temp))
>>                     && (GET_MODE (SUBREG_REG (target)) == GET_MODE
>> (SUBREG_REG (temp))))
>
> Looks like a wrong order of the predicates in any case, first you should
> check if it is a SUBREG, then SUBREG_PROMOTED_VAR_P and only then
> SUBREG_PROMOTED_GET.  Also, the extra ()s around single line conditions
> are unnecessary.

This comment applies to the original code, not the patched line, I guess.

Uros.