Re: [PATCH] Fix another reg-stack recovery bug (PR target/84828)

[Jakub Jelinek]

On Tue, Mar 13, 2018 at 04:59:58AM -0700, H.J. Lu wrote:
> >> The testcase still ICEs on i686-linux (preexisting bug, ICEs the same
> >> without this patch or even before the previous patch), will handle that
> >> tomorrow.

> I got:

See the above?
Or do you get this with -m64 too, rather than -m32 only?

Jakub

Re: [PATCH] Fix another reg-stack recovery bug (PR target/84828)

[H.J. Lu]

On Tue, Mar 13, 2018 at 5:02 AM, Jakub Jelinek  wrote:
> On Tue, Mar 13, 2018 at 04:59:58AM -0700, H.J. Lu wrote:
>> >> The testcase still ICEs on i686-linux (preexisting bug, ICEs the same
>> >> without this patch or even before the previous patch), will handle that
>> >> tomorrow.
>
>> I got:
>
> See the above?
> Or do you get this with -m64 too, rather than -m32 only?
>

I got this for -m32, -m64 and -mx32.

-- 
H.J.

Re: [PATCH] Fix another reg-stack recovery bug (PR target/84828)

[H.J. Lu]

On Tue, Mar 13, 2018 at 12:55 AM, Uros Bizjak  wrote:
> On Mon, Mar 12, 2018 at 10:10 PM, Jakub Jelinek  wrote:
>> Hi!
>>
>> As Martin reported, the same testcase added recently ICEs differently
>> with different options on x86_64-linux, the problem is that we
>> sometimes emit insns before a CODE_LABEL of the next bb rather than after
>> the last insn in the previous bb, and expect we can just fix up BB_END
>> of the previous bb; fortunately that doesn't change BB_HEAD of the next
>> bb, but still we need to at least update INSN_BLOCK and make sure df knows
>> that too.
>>
>> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
>> trunk?
>>
>> The testcase still ICEs on i686-linux (preexisting bug, ICEs the same
>> without this patch or even before the previous patch), will handle that
>> tomorrow.
>>
>> 2018-03-12  Jakub Jelinek  
>>
>> PR target/84828
>> * reg-stack.c (change_stack): Change update_end var from int to
>> rtx_insn *, if non-NULL don't update just BB_END (current_block), but
>> also call set_block_for_insn on the newly added insns and rescan.
>>
>> * g++.dg/ext/pr84828.C: New test.
>
> LGTM.
>
> Uros.
>

I got:

spawn -ignore SIGHUP
/export/gnu/import/git/gcc-test/bld/gcc/testsuite/g++/../../xg++
-B/export/gnu/import/git/gcc-test/bld/gcc/testsuite/g++/../../
/export/gnu/import/git/gcc-test/src-trunk/gcc/testsuite/g++.dg/ext/pr84828.C
-fno-diagnostics-show-caret -fdiagnostics-color=never -nostdinc++
-I/export/gnu/import/git/gcc-test/bld/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu
-I/export/gnu/import/git/gcc-test/bld/x86_64-pc-linux-gnu/libstdc++-v3/include
-I/export/gnu/import/git/gcc-test/src-trunk/libstdc++-v3/libsupc++
-I/export/gnu/import/git/gcc-test/src-trunk/libstdc++-v3/include/backward
-I/export/gnu/import/git/gcc-test/src-trunk/libstdc++-v3/testsuite/util
-fmessage-length=0 -std=gnu++98 -Og -mno-sse2 -S -o pr84828.s^M
/export/gnu/import/git/gcc-test/src-trunk/gcc/testsuite/g++.dg/ext/pr84828.C:
In function 'void foo(float, double)':^M
/export/gnu/import/git/gcc-test/src-trunk/gcc/testsuite/g++.dg/ext/pr84828.C:10:35:
error: output constraint 0 must specify a single register^M
during RTL pass: stack^M
/export/gnu/import/git/gcc-test/src-trunk/gcc/testsuite/g++.dg/ext/pr84828.C:13:1:
internal compiler error: in move_for_stack_reg, at reg-stack.c:1108^M
0xeaf075 move_for_stack_reg^M
../../src-trunk/gcc/reg-stack.c:1108^M
0xeb26ea subst_stack_regs^M
../../src-trunk/gcc/reg-stack.c:2438^M
0xeb2950 convert_regs_1^M
../../src-trunk/gcc/reg-stack.c:3072^M
0xeb2950 convert_regs_2^M
../../src-trunk/gcc/reg-stack.c:3207^M
0xeb4798 convert_regs^M
../../src-trunk/gcc/reg-stack.c:3242^M
0xeb4798 reg_to_stack^M
../../src-trunk/gcc/reg-stack.c:3367^M
0xeb4798 rest_of_handle_stack_regs^M
../../src-trunk/gcc/reg-stack.c:3422^M
0xeb4798 execute^M
../../src-trunk/gcc/reg-stack.c:3453^M
Please submit a full bug report,^M
with preprocessed source if appropriate.^M
Please include the complete backtrace with any bug report.^M
See  for instructions.^M
compiler exited with status 1
FAIL: g++.dg/ext/pr84828.C  -std=gnu++98 (internal compiler error)
PASS: g++.dg/ext/pr84828.C  -std=gnu++98  (test for errors, line 10)
FAIL: g++.dg/ext/pr84828.C  -std=gnu++98 (test for excess errors)

-- 
H.J.

Re: [PATCH] Fix another reg-stack recovery bug (PR target/84828)

[Uros Bizjak]

On Mon, Mar 12, 2018 at 10:10 PM, Jakub Jelinek  wrote:
> Hi!
>
> As Martin reported, the same testcase added recently ICEs differently
> with different options on x86_64-linux, the problem is that we
> sometimes emit insns before a CODE_LABEL of the next bb rather than after
> the last insn in the previous bb, and expect we can just fix up BB_END
> of the previous bb; fortunately that doesn't change BB_HEAD of the next
> bb, but still we need to at least update INSN_BLOCK and make sure df knows
> that too.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
>
> The testcase still ICEs on i686-linux (preexisting bug, ICEs the same
> without this patch or even before the previous patch), will handle that
> tomorrow.
>
> 2018-03-12  Jakub Jelinek  
>
> PR target/84828
> * reg-stack.c (change_stack): Change update_end var from int to
> rtx_insn *, if non-NULL don't update just BB_END (current_block), but
> also call set_block_for_insn on the newly added insns and rescan.
>
> * g++.dg/ext/pr84828.C: New test.

LGTM.

Uros.

> --- gcc/reg-stack.c.jj  2018-03-12 15:06:21.565800672 +0100
> +++ gcc/reg-stack.c 2018-03-12 15:06:31.167802560 +0100
> @@ -2489,7 +2489,7 @@ change_stack (rtx_insn *insn, stack_ptr
>   enum emit_where where)
>  {
>int reg;
> -  int update_end = 0;
> +  rtx_insn *update_end = NULL;
>int i;
>
>/* Stack adjustments for the first insn in a block update the
> @@ -2511,7 +2511,7 @@ change_stack (rtx_insn *insn, stack_ptr
>if (where == EMIT_AFTER)
>  {
>if (current_block && BB_END (current_block) == insn)
> -   update_end = 1;
> +   update_end = insn;
>insn = NEXT_INSN (insn);
>  }
>
> @@ -2686,7 +2686,16 @@ change_stack (rtx_insn *insn, stack_ptr
>  }
>
>if (update_end)
> -BB_END (current_block) = PREV_INSN (insn);
> +{
> +  for (update_end = NEXT_INSN (update_end); update_end != insn;
> +  update_end = NEXT_INSN (update_end))
> +   {
> + set_block_for_insn (update_end, current_block);
> + if (INSN_P (update_end))
> +   df_insn_rescan (update_end);
> +   }
> +  BB_END (current_block) = PREV_INSN (insn);
> +}
>  }
>
>  /* Print stack configuration.  */
> --- gcc/testsuite/g++.dg/ext/pr84828.C.jj   2018-03-12 15:13:50.589882983 
> +0100
> +++ gcc/testsuite/g++.dg/ext/pr84828.C  2018-03-12 15:14:06.670885916 +0100
> @@ -0,0 +1,13 @@
> +// PR target/84828
> +// { dg-do compile { target i?86-*-* x86_64-*-* } }
> +// { dg-options "-Og -mno-sse2" }
> +
> +void
> +foo (float b, double c)
> +{
> +  for (int e = 0; e < 2; e++)
> +{
> +  asm volatile ("" : "+f" (c));// { dg-error "must specify a single 
> register" }
> +  asm ("" : "+rm" (c = b));
> +}
> +}
>
> Jakub