On Wed, 5 Jan 2022, Jakub Jelinek wrote:
> Hi!
>
> On the following testcase, RTL ifcvt sees then_bb
> (note 7 6 8 3 [bb 3] NOTE_INSN_BASIC_BLOCK)
> (insn 8 7 9 3 (set (mem/c:SI (symbol_ref:DI ("b") [flags 0x2] <var_decl
> 0x7fdccf5b0cf0 b>) [1 b+0 S4 A32])
> (const_int 1 [0x1])) "pr103908.c":6:7 81 {*movsi_internal}
> (nil))
> (jump_insn 9 8 13 3 (parallel [
> (asm_operands/v ("# insn 1") ("") 0 []
> []
> [
> (label_ref:DI 21)
> ] pr103908.c:7)
> (clobber (reg:CC 17 flags))
> ]) "pr103908.c":7:5 -1
> (expr_list:REG_UNUSED (reg:CC 17 flags)
> (nil))
> -> 21)
> and similarly else_bb (just with a different asm_operands template).
> It checks that those basic blocks have a single successor and
> uses last_active_insn which intentionally skips over JUMP_INSNs, sees
> both basic blocks contain the same set and merges them (or if the
> sets are different, attempts some other noce optimization).
> But we can't assume that the jump, even when it has only a single successor,
> has no side-effects.
>
> The following patch fixes it by checking specially for asm goto, but
> I wonder if it wouldn't be better to
> if (JUMP_P (BB_END (test_bb)) && !onlyjump_p (BB_END (test_bb)))
> return false;
> and give up on any other hypothetical single successor jumps.
>
> The patch below has been bootstrapped/regtested on x86_64-linux and
> i686-linux, I could bootstrap the !onlyjump_p version too...
I'd prefer checking with onlyjump_p. OK with that change.
Thanks,
Richard.
> 2022-01-05 Jakub Jelinek <ja...@redhat.com>
>
> PR rtl-optimization/103908
> * ifcvt.c (bb_valid_for_noce_process_p): Punt on bbs ending with
> asm goto.
>
> * gcc.target/i386/pr103908.c: New test.
>
> --- gcc/ifcvt.c.jj 2022-01-03 10:40:42.919140216 +0100
> +++ gcc/ifcvt.c 2022-01-04 23:50:40.962529052 +0100
> @@ -3064,6 +3064,13 @@ bb_valid_for_noce_process_p (basic_block
>
> if (!insn_valid_noce_process_p (last_insn, cc))
> return false;
> +
> + /* Punt for blocks ending with asm goto, last_active_insn
> + ignores JUMP_INSNs. */
> + if (JUMP_P (BB_END (test_bb))
> + && asm_noperands (PATTERN (BB_END (test_bb))) >= 0)
> + return false;
> +
> last_set = single_set (last_insn);
>
> rtx x = SET_DEST (last_set);
> --- gcc/testsuite/gcc.target/i386/pr103908.c.jj 2022-01-04
> 23:58:02.992341275 +0100
> +++ gcc/testsuite/gcc.target/i386/pr103908.c 2022-01-04 23:57:39.399671539
> +0100
> @@ -0,0 +1,24 @@
> +/* PR rtl-optimization/103908 */
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -fdisable-tree-cselim -fno-tree-sink" } */
> +/* { dg-final { scan-assembler "# insn 1" } } */
> +/* { dg-final { scan-assembler "# insn 2" } } */
> +
> +int a, b;
> +
> +void
> +foo (void)
> +{
> + if (a)
> + {
> + b = 1;
> + asm goto ("# insn 1" : : : : lab1);
> + lab1:;
> + }
> + else
> + {
> + b = 1;
> + asm goto ("# insn 2" : : : : lab2);
> + lab2:;
> + }
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)
