public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] ree: Fix -fcompare-debug issues in combine_reaching_defs [PR108573]
@ 2023-02-01 11:37 Jakub Jelinek
  2023-02-01 11:42 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2023-02-01 11:37 UTC (permalink / raw)
  To: Jeff Law, Richard Biener, Eric Botcazou; +Cc: gcc-patches

Hi!

The PR78437 r7-4871 changes made combine_reaching_defs punt on
WORD_REGISTER_OPERATIONS targets if a setter of smaller than word
register has wider uses.  This unfortunately breaks -fcompare-debug,
because if such a use appears only in DEBUG_INSN(s), while all other
uses aren't wider than the setter, we can REE optimize it without -g
and not with -g.

Such decisions shouldn't be based on debug instructions.  We could try
to reset them or adjust in some other way after we decide to perform the
change, but at least on the testcase which used to fail on riscv64-linux
the
(debug_insn 8 7 9 2 (var_location:HI s (minus:HI (subreg:HI (and:DI (reg:DI 10 a0 [160])
                (const_int 1 [0x1])) 0)
        (subreg:HI (ashiftrt:DI (reg/v:DI 9 s1 [orig:151 l ] [151])
                (debug_expr:SI D#1)) 0))) "pr108573.c":12:5 -1
     (nil))
clearly doesn't care about the upper bits and I have hard time imaging how
could one end up with DEBUG_INSN which actually cares about those upper
bits.

So, the following patch just ignores uses on DEBUG_INSNs in this case,
if we run into something where we'd need to do something further later on,
let's deal with it when we have a testcase for it.

Bootstrapped/regtested on x86_64-linux and i686-linux (admittedly
non-WORD_REGISTER_OPERATIONS targets) and tested using a cross to
riscv64-linux on the testcase, ok for trunk?

2023-02-01  Jakub Jelinek  <jakub@redhat.com>

	PR debug/108573
	* ree.cc (combine_reaching_defs): Don't return false for paradoxical
	subregs in DEBUG_INSNs.

	* gcc.dg/pr108573.c: New test.

--- gcc/ree.cc.jj	2023-02-01 10:19:22.833436546 +0100
+++ gcc/ree.cc	2023-02-01 12:25:57.468252609 +0100
@@ -875,7 +875,8 @@ combine_reaching_defs (ext_cand *cand, c
 
 	  for (df_link *use = uses; use; use = use->next)
 	    if (paradoxical_subreg_p (GET_MODE (*DF_REF_LOC (use->ref)),
-				      GET_MODE (SET_DEST (*dest_sub_rtx))))
+				      GET_MODE (SET_DEST (*dest_sub_rtx)))
+		&& !DEBUG_INSN_P (DF_REF_INSN (use->ref)))
 	      return false;
 	}
 
@@ -963,7 +964,8 @@ combine_reaching_defs (ext_cand *cand, c
 	      rtx dest2 = SET_DEST (*dest_sub_rtx2);
 	      for (use = uses; use; use = use->next)
 		if (paradoxical_subreg_p (GET_MODE (*DF_REF_LOC (use->ref)),
-					  GET_MODE (dest2)))
+					  GET_MODE (dest2))
+		    && !DEBUG_INSN_P (DF_REF_INSN (use->ref)))
 		  break;
 	      if (use)
 		break;
--- gcc/testsuite/gcc.dg/pr108573.c.jj	2023-02-01 12:15:20.476577340 +0100
+++ gcc/testsuite/gcc.dg/pr108573.c	2023-02-01 12:15:03.044832520 +0100
@@ -0,0 +1,18 @@
+/* PR debug/108573 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcompare-debug" } */
+
+unsigned g;
+
+int bar (void);
+int baz (int);
+
+void
+foo (unsigned short s, long l)
+{
+  unsigned u = bar ();
+  s &= __builtin_add_overflow_p (0, u, 0);
+  s %= g;
+  s -= l >> s;
+  baz (s);
+}

	Jakub


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] ree: Fix -fcompare-debug issues in combine_reaching_defs [PR108573]
  2023-02-01 11:37 [PATCH] ree: Fix -fcompare-debug issues in combine_reaching_defs [PR108573] Jakub Jelinek
@ 2023-02-01 11:42 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2023-02-01 11:42 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jeff Law, Eric Botcazou, gcc-patches

On Wed, 1 Feb 2023, Jakub Jelinek wrote:

> Hi!
> 
> The PR78437 r7-4871 changes made combine_reaching_defs punt on
> WORD_REGISTER_OPERATIONS targets if a setter of smaller than word
> register has wider uses.  This unfortunately breaks -fcompare-debug,
> because if such a use appears only in DEBUG_INSN(s), while all other
> uses aren't wider than the setter, we can REE optimize it without -g
> and not with -g.
> 
> Such decisions shouldn't be based on debug instructions.  We could try
> to reset them or adjust in some other way after we decide to perform the
> change, but at least on the testcase which used to fail on riscv64-linux
> the
> (debug_insn 8 7 9 2 (var_location:HI s (minus:HI (subreg:HI (and:DI (reg:DI 10 a0 [160])
>                 (const_int 1 [0x1])) 0)
>         (subreg:HI (ashiftrt:DI (reg/v:DI 9 s1 [orig:151 l ] [151])
>                 (debug_expr:SI D#1)) 0))) "pr108573.c":12:5 -1
>      (nil))
> clearly doesn't care about the upper bits and I have hard time imaging how
> could one end up with DEBUG_INSN which actually cares about those upper
> bits.
> 
> So, the following patch just ignores uses on DEBUG_INSNs in this case,
> if we run into something where we'd need to do something further later on,
> let's deal with it when we have a testcase for it.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux (admittedly
> non-WORD_REGISTER_OPERATIONS targets) and tested using a cross to
> riscv64-linux on the testcase, ok for trunk?

OK.

Richard.

> 2023-02-01  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR debug/108573
> 	* ree.cc (combine_reaching_defs): Don't return false for paradoxical
> 	subregs in DEBUG_INSNs.
> 
> 	* gcc.dg/pr108573.c: New test.
> 
> --- gcc/ree.cc.jj	2023-02-01 10:19:22.833436546 +0100
> +++ gcc/ree.cc	2023-02-01 12:25:57.468252609 +0100
> @@ -875,7 +875,8 @@ combine_reaching_defs (ext_cand *cand, c
>  
>  	  for (df_link *use = uses; use; use = use->next)
>  	    if (paradoxical_subreg_p (GET_MODE (*DF_REF_LOC (use->ref)),
> -				      GET_MODE (SET_DEST (*dest_sub_rtx))))
> +				      GET_MODE (SET_DEST (*dest_sub_rtx)))
> +		&& !DEBUG_INSN_P (DF_REF_INSN (use->ref)))
>  	      return false;
>  	}
>  
> @@ -963,7 +964,8 @@ combine_reaching_defs (ext_cand *cand, c
>  	      rtx dest2 = SET_DEST (*dest_sub_rtx2);
>  	      for (use = uses; use; use = use->next)
>  		if (paradoxical_subreg_p (GET_MODE (*DF_REF_LOC (use->ref)),
> -					  GET_MODE (dest2)))
> +					  GET_MODE (dest2))
> +		    && !DEBUG_INSN_P (DF_REF_INSN (use->ref)))
>  		  break;
>  	      if (use)
>  		break;
> --- gcc/testsuite/gcc.dg/pr108573.c.jj	2023-02-01 12:15:20.476577340 +0100
> +++ gcc/testsuite/gcc.dg/pr108573.c	2023-02-01 12:15:03.044832520 +0100
> @@ -0,0 +1,18 @@
> +/* PR debug/108573 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fcompare-debug" } */
> +
> +unsigned g;
> +
> +int bar (void);
> +int baz (int);
> +
> +void
> +foo (unsigned short s, long l)
> +{
> +  unsigned u = bar ();
> +  s &= __builtin_add_overflow_p (0, u, 0);
> +  s %= g;
> +  s -= l >> s;
> +  baz (s);
> +}
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-02-01 11:42 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-01 11:37 [PATCH] ree: Fix -fcompare-debug issues in combine_reaching_defs [PR108573] Jakub Jelinek
2023-02-01 11:42 ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).