Janis Johnson <[EMAIL PROTECTED]> writes:
> On Mon, 2007-09-17 at 23:41 +0200, Andreas Schwab wrote:
>> Janis Johnson <[EMAIL PROTECTED]> writes:
>> 
>> > Bootstrap of powerpc64-linux fails building libgfortran with:
>> >
>> > /home/janis/gcc_trunk_anonsvn/gcc/libgfortran/io/read.c: In function 
>> > 'set_integer':
>> > /home/janis/gcc_trunk_anonsvn/gcc/libgfortran/io/read.c:81: internal 
>> > compiler error: in set_variable_part, at var-tracking.c:2381
>> > Please submit a full bug report, with preprocessed source if appropriate.
>> > See <http://gcc.gnu.org/bugs.html> for instructions.
>> >
>> > My last successful build was revision 128522; earliest known break
>> > was 128536, still breaks with 128551.
>> 
>> Bisection has identified this change:
>> 
>> 2007-09-16  Richard Sandiford  <[EMAIL PROTECTED]>
>> 
>>      * dse.c (find_shift_sequence): Allow word as well as subword shifts.
>>      Do the tentative shift expansion with the DF_NO_INSN_RESCAN flag set.
>>      Fix the call to insn_rtx_cost.  Skip access sizes that require a
>>      real truncation of the store register.  Use convert_move instead
>>      of gen_lowpart when narrowing the result.
>>      (replace_read): Use convert_move instead of gen_lowpart when
>>      narrowing the store rhs.
>
> My hunt just found the same patch  Here's a small test that fails
> with cc1 for powerpc64-linux with "-O2 -g -m64 -mstrict-align":
> -------------
> extern void *memcpy (void *, const void *, signed long);
>
> void
> set_integer (void *dest, __int128_t value, int length)
> {
>   switch (length)
>     {
>     case 16:
>       {
>          __int128_t tmp = value;
>          memcpy (dest, (void *) &tmp, length);
>       }
>       break;
>     case 1:
>       {
>         signed char tmp = value;
>         memcpy (dest, (void *) &tmp, length);
>       }
>       break;
>     }
> }
> -----------
> Janis

I think this is a latent bug in var-tracking.c.  It's getting confused
by the negative offsets in:

(insn:HI 17 47 19 4 /tmp/foo.c:11 (set (reg:DI 26 26 [orig:124 tmp+-7 ] [124])
        (lshiftrt:DI (reg:DI 31 31 [orig:141 value ] [141])
            (const_int 56 [0x38]))) 292 {*lshrdi3_internal1} (nil))

(insn:HI 19 17 21 4 /tmp/foo.c:11 (set (reg:DI 27 27 [orig:125 tmp+-6 ] [125])
        (lshiftrt:DI (reg:DI 31 31 [orig:141 value ] [141])
            (const_int 48 [0x30]))) 292 {*lshrdi3_internal1} (nil))

where the registers have come from paradoxical subregs of QImode registers.
(DSE itself didn't create those; combine did.  DSE just created DImode
shift instructions.)

FWIW, the part of the patch that exposed the bug as the change from
"access_bytes < UNITS_PER_WORD" to "access_bytes <= UNITS_PER_WORD".
Your testcase passes if we change that back.

>From my POV of view, feel free to commit that change until the underlying
var-tracking bug is fixed.  Alternatively, I think it'd be OK to revert
the whole of my patch _and_ revert Kenny's patch.  (Please don't just
revert mine, as it would reintroduce a bootstrap failure on MIPS targets.)

I'm afraid I won't have time to work on this myself.  Hopefully Kenny
will.  (At the end of the day, I was only trying to fix MIPS breakage
in the original DSE patch, rather than asking Kenny to it for me. ;))

Richard

Reply via email to