Re: [PR69634] fix debug_insn-inconsistent REG_N_CALLS_CROSSED

[Andreas Schwab]

Alexandre Oliva  writes:

> diff --git a/gcc/testsuite/gcc.dg/pr69634.c b/gcc/testsuite/gcc.dg/pr69634.c
> new file mode 100644
> index 000..837bd57
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr69634.c
> @@ -0,0 +1,22 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-dce -fschedule-insns -fno-tree-vrp 
> -fcompare-debug" } */
> +/* { dg-additional-options "-Wno-psabi -mno-sse" { target i?86-*-* 
> x86_64-*-* } } */
> +/* { dg-additional-options "-m32" { target x86_64-*-* } } */
> +
> +typedef unsigned short u16;
> +typedef short v16u16 __attribute__ ((vector_size (16)));
> +typedef unsigned v16u32 __attribute__ ((vector_size (16)));
> +typedef unsigned long long v16u64 __attribute__ ((vector_size (16)));
> +
> +u16
> +foo(u16 u16_1, v16u16 v16u16_0, v16u32 v16u64_0, v16u16 v16u16_1, v16u32 
> v16u32_1, v16u64 v16u64_1)

On powerpc -m32:

FAIL: gcc.dg/pr69634.c (test for excess errors)
Excess errors:
/daten/gcc/gcc-20160307/gcc/testsuite/gcc.dg/pr69634.c:11:1: warning: GCC 
vector passed by reference: non-standard ABI extension with no compatibility 
guarantee

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PR69634] fix debug_insn-inconsistent REG_N_CALLS_CROSSED

[Jeff Law]

On 02/06/2016 03:06 AM, Alexandre Oliva wrote:

The testcase has a debug insn referencing a pseudo right before an
insn that modifies the pseudo.

Without debug insns, REG_N_CALLS_CROSSED was zero for that pseudo, so
sched_analyze_reg added a dep between the pseudo setter and an earlier
(lib)call.

With debug insns, we miscomputed REG_N_CALLS_CROSSED as nonzero
because of the debug insn, and then no dep was added between the two
insns.  This was enough to change sched1's decisions about where to
place the pseudo setter.

REG_N_CALLS_CROSSED is computed by both regstat_bb_compute_ri and
regstat_bb_compute_calls_crossed, but although the former skipped
debug insns, the latter didn't.

Fixing this inconsistency was enough to fix the -fcompare-debug error.

Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  Ok to install?


for  gcc/ChangeLog

PR target/69634
* regstat.c (regstat_bb_compute_calls_crossed): Disregard
debug insns.

for  gcc/testsuite/ChangeLog

PR target/69634
* gcc.dg/pr69634.c: New.
I removed the explicit -m32.  It was there merely to try and exercise 
the test, even on an x86-64 configured toolchain.  As Uros noted, a 
standard multilib of x86-64 will test -m32, so the explicit -m32 isn't 
needed and it is in fact harmful.


Committed to the trunk with that change.  I'm going to add 4.9/5 
regression markers to the BZ since this affects those releases as well.


Jeff

Re: [PR69634] fix debug_insn-inconsistent REG_N_CALLS_CROSSED

[Jeff Law]

On 02/06/2016 03:06 AM, Alexandre Oliva wrote:

The testcase has a debug insn referencing a pseudo right before an
insn that modifies the pseudo.

Without debug insns, REG_N_CALLS_CROSSED was zero for that pseudo, so
sched_analyze_reg added a dep between the pseudo setter and an earlier
(lib)call.

With debug insns, we miscomputed REG_N_CALLS_CROSSED as nonzero
because of the debug insn, and then no dep was added between the two
insns.  This was enough to change sched1's decisions about where to
place the pseudo setter.

REG_N_CALLS_CROSSED is computed by both regstat_bb_compute_ri and
regstat_bb_compute_calls_crossed, but although the former skipped
debug insns, the latter didn't.

Fixing this inconsistency was enough to fix the -fcompare-debug error.

Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  Ok to install?


for  gcc/ChangeLog

PR target/69634
* regstat.c (regstat_bb_compute_calls_crossed): Disregard
debug insns.

for  gcc/testsuite/ChangeLog

PR target/69634
* gcc.dg/pr69634.c: New.

Code is fine.  Testcase needs tweaking per Uros's comments in the BZ:

  Please don't use explicit -m32. If the test is valid only for 32bit
  target, you should use "target ia32", otherwise just leave it out.
  32bit multilib tests will add -m32 automatically.

  Explicit -m32 will probably fail with x32, which is x86_64 target,
  too.

With the test fixed this is fine.

jeff

Re: [PR69634] fix debug_insn-inconsistent REG_N_CALLS_CROSSED

[Jakub Jelinek]

On Sat, Feb 06, 2016 at 08:06:18AM -0200, Alexandre Oliva wrote:
> The testcase has a debug insn referencing a pseudo right before an
> insn that modifies the pseudo.
> 
> Without debug insns, REG_N_CALLS_CROSSED was zero for that pseudo, so
> sched_analyze_reg added a dep between the pseudo setter and an earlier
> (lib)call.
> 
> With debug insns, we miscomputed REG_N_CALLS_CROSSED as nonzero
> because of the debug insn, and then no dep was added between the two
> insns.  This was enough to change sched1's decisions about where to
> place the pseudo setter.
> 
> REG_N_CALLS_CROSSED is computed by both regstat_bb_compute_ri and
> regstat_bb_compute_calls_crossed, but although the former skipped
> debug insns, the latter didn't.
> 
> Fixing this inconsistency was enough to fix the -fcompare-debug error.
> 
> Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  Ok to install?

Ok, with nit; thanks.

> for  gcc/ChangeLog
> 
>   PR target/69634
>   * regstat.c (regstat_bb_compute_calls_crossed): Disregard
>   debug insns.
> 
> for  gcc/testsuite/ChangeLog
> 
>   PR target/69634
>   * gcc.dg/pr69634.c: New.

> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr69634.c
> @@ -0,0 +1,22 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-dce -fschedule-insns -fno-tree-vrp 
> -fcompare-debug" } */
> +/* { dg-additional-options "-Wno-psabi -mno-sse" { target i?86-*-* 
> x86_64-*-* } } */

> +/* { dg-additional-options "-m32" { target x86_64-*-* } } */

Please remove the above line, -m32 should come solely from the user, and enough
people either build i686-linux compilers or test with
RUNTESTFLAGS='--target_board=unix\{-m64,-m32\}' that a regression in here
would be noticed soon.

Jakub

[PR69634] fix debug_insn-inconsistent REG_N_CALLS_CROSSED

[Alexandre Oliva]

The testcase has a debug insn referencing a pseudo right before an
insn that modifies the pseudo.

Without debug insns, REG_N_CALLS_CROSSED was zero for that pseudo, so
sched_analyze_reg added a dep between the pseudo setter and an earlier
(lib)call.

With debug insns, we miscomputed REG_N_CALLS_CROSSED as nonzero
because of the debug insn, and then no dep was added between the two
insns.  This was enough to change sched1's decisions about where to
place the pseudo setter.

REG_N_CALLS_CROSSED is computed by both regstat_bb_compute_ri and
regstat_bb_compute_calls_crossed, but although the former skipped
debug insns, the latter didn't.

Fixing this inconsistency was enough to fix the -fcompare-debug error.

Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  Ok to install?


for  gcc/ChangeLog

PR target/69634
* regstat.c (regstat_bb_compute_calls_crossed): Disregard
debug insns.

for  gcc/testsuite/ChangeLog

PR target/69634
* gcc.dg/pr69634.c: New.
---
 gcc/regstat.c  |2 +-
 gcc/testsuite/gcc.dg/pr69634.c |   22 ++
 2 files changed, 23 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr69634.c

diff --git a/gcc/regstat.c b/gcc/regstat.c
index af5e475..c05b69f 100644
--- a/gcc/regstat.c
+++ b/gcc/regstat.c
@@ -444,7 +444,7 @@ regstat_bb_compute_calls_crossed (unsigned int bb_index, 
bitmap live)
   struct df_insn_info *insn_info = DF_INSN_INFO_GET (insn);
   unsigned int regno;
 
-  if (!INSN_P (insn))
+  if (!NONDEBUG_INSN_P (insn))
continue;
 
   /* Process the defs.  */
diff --git a/gcc/testsuite/gcc.dg/pr69634.c b/gcc/testsuite/gcc.dg/pr69634.c
new file mode 100644
index 000..837bd57
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr69634.c
@@ -0,0 +1,22 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-dce -fschedule-insns -fno-tree-vrp -fcompare-debug" 
} */
+/* { dg-additional-options "-Wno-psabi -mno-sse" { target i?86-*-* x86_64-*-* 
} } */
+/* { dg-additional-options "-m32" { target x86_64-*-* } } */
+
+typedef unsigned short u16;
+typedef short v16u16 __attribute__ ((vector_size (16)));
+typedef unsigned v16u32 __attribute__ ((vector_size (16)));
+typedef unsigned long long v16u64 __attribute__ ((vector_size (16)));
+
+u16
+foo(u16 u16_1, v16u16 v16u16_0, v16u32 v16u64_0, v16u16 v16u16_1, v16u32 
v16u32_1, v16u64 v16u64_1)
+{
+  v16u64_1 /= (v16u64){~v16u32_1[1]};
+  u16_1 = 0;
+  u16_1 /= v16u32_1[2];
+  v16u64_1 -= (v16u64) v16u16_1;
+  u16_1 >>= 1;
+  u16_1 -= ~0;
+  v16u16_1 /= (v16u16){~u16_1, 1 - v16u64_0[0], 0xffb6};
+  return u16_1 + v16u16_0[1] + v16u16_1[3] + v16u64_1[0] + v16u64_1[1];
+}


-- 
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer