On Fri, 25 Feb 2022, Jakub Jelinek wrote:
> Hi!
>
> The following testcase is miscompiled on ia32 at -O2, because
> when expand_SPACESHIP is called, we have pending stack adjustment
> from the foo call right before it.
> Now, ix86_expand_fp_spaceship uses emit_jump_insn several times
> but then emit_jump also several times. While emit_jump_insn doesn't
> do do_pending_stack_adjust (), emit_jump does, so we end up with:
> ...
> 8: call [`_Z3foodl'] argc:0x10
> REG_CALL_DECL `_Z3foodl'
> 9: r88:DF=[`a']
> 10: r89:HI=unspec[cmp(r88:DF,0.0)] 25
> 11: flags:CC=unspec[r89:HI] 26
> 12: pc={(unordered(flags:CCFP,0))?L27:pc}
> REG_BR_PROB 536868
> 66: NOTE_INSN_BASIC_BLOCK 4
> 13: pc={(uneq(flags:CCFP,0))?L19:pc}
> REG_BR_PROB 214748364
> 67: NOTE_INSN_BASIC_BLOCK 5
> 14: pc={(flags:CCFP>0)?L23:pc}
> REG_BR_PROB 536870916
> 68: NOTE_INSN_BASIC_BLOCK 6
> 15: r86:SI=0xffffffffffffffff
> 16: {sp:SI=sp:SI+0x10;clobber flags:CC;}
> REG_ARGS_SIZE 0
> 17: pc=L29
> 18: barrier
> 19: L19:
> 69: NOTE_INSN_BASIC_BLOCK 7
> ...
> The sp += 16 pending stuck adjust was emitted in the middle of the
> sequence and is effective only for the single case of the 4 possibilities
> where .SPACESHIP returns -1, in all other cases the stack isn't adjusted
> and so we ICE during dwarf2cfi.
>
> Now, we could either call do_pending_stack_adjust in
> ix86_expand_fp_spaceship, or use there calls that actually don't call
> do_pending_stack_adjust (but having the stack adjustment across branches is
> generally undesirable), or we can call it in expand_SPACESHIP for all
> targets (note, just i386 currently implements it).
> I chose the generic code because e.g. expand_{addsub,neg,mul}_overflow
> in the same file also call do_pending_stack_adjust in internal-fn.cc for the
> same reasons, that it is expected that most if not all targets will expand
> those through jumps and we don't want all of the targets to need to deal
> with that.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
> Or do you prefer it in ix86_expand_fp_spaceship instead?
No, as you say others will likely repeat the mistake otherwise.
Richard.
> 2022-02-24 Jakub Jelinek <ja...@redhat.com>
>
> PR middle-end/104679
> * internal-fn.cc (expand_SPACESHIP): Call do_pending_stack_adjust.
>
> * g++.dg/torture/pr104679.C: New test.
>
> --- gcc/internal-fn.cc.jj 2022-02-11 13:51:07.757597854 +0100
> +++ gcc/internal-fn.cc 2022-02-24 17:46:01.476722703 +0100
> @@ -4437,6 +4437,8 @@ expand_SPACESHIP (internal_fn, gcall *st
> tree rhs2 = gimple_call_arg (stmt, 1);
> tree type = TREE_TYPE (rhs1);
>
> + do_pending_stack_adjust ();
> +
> rtx target = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
> rtx op1 = expand_normal (rhs1);
> rtx op2 = expand_normal (rhs2);
> --- gcc/testsuite/g++.dg/torture/pr104679.C.jj 2022-02-24
> 17:48:08.309959261 +0100
> +++ gcc/testsuite/g++.dg/torture/pr104679.C 2022-02-24 17:48:00.226071655
> +0100
> @@ -0,0 +1,22 @@
> +// PR middle-end/104679
> +// { dg-do compile }
> +
> +struct A { ~A (); };
> +void foo (double, long);
> +void bar ();
> +double a;
> +long b;
> +
> +void
> +baz ()
> +{
> + foo (a, b);
> + if (a == 0.0)
> + ;
> + else
> + while (a > 0.0)
> + {
> + A c;
> + bar ();
> + }
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)
