Re: [PATCH, rtl] Fix PR84878: Segmentation fault in add_cross_iteration_register_deps

[Peter Bergner <bergner at vnet dot ibm dot com>]

On 4/2/18 9:21 AM, Alexander Monakov wrote:
> On Tue, 27 Mar 2018, Richard Biener wrote:
>> If they only appear in the exit/entry block ignoring them should be safe.
>>
>> But who knows...
> 
> Roman and I discussed a related problem a few weeks ago, so here's my 2c.
> As I don't have any special DF knowledge, this is merely my understanding.
> 
> (apropos i: SMS uses sched-deps for intra-loop deps, and then separately uses
> DF for cross-iteration deps, which means that it should be ready for surprises
> when the two scanners are not 100% in sync)
> 
> (apropos ii: given the flexibility of RTL, it would have been really nice
> if there were no implicit cc0-like uses that need to be special-cased in DF,
> sched-deps and other scanners)
> 
> In this case I believe it's fine to skip processing of r_use when the associated
> BB is not the loop BB (i.e. 'if (DF_REF_BB (r_use->ref) != g->bb)' as Richard
> suggested), but I'm concerned that skipping it when the artificial's use BB
> corresponds to loop BB goes from ICE to wrong-code. It should be detected
> earlier, in sms_schedule (see the comment starting with "Don't handle BBs with
> calls or barriers").

Ok, getting back to this, I see the r_use->ref for the global reg that
is exposed all the way to the exit block is marked as artificial, so
DF_REF_IS_ARTIFICIAL (r_use->ref) is true.  So using richi and Alexander's
suggestion of DF_REF_BB(), I cobbled the change below.  I added an assert
to catch any cases where the artificial use is the same as g->bb or where
we still might have no insn info.  I never hit the assert in either my
bootstrap or in my testsuite runs.  Is something like the following what
we want?

This did pass bootstrap and regtesting with no regressions on powerpc64-linux
running the testsuite in both 32-bit and 64-bit modes.

Peter


gcc/
	PR rtl-optimization/84878
	* ddg.c (add_cross_iteration_register_deps): Use DF_REF_BB to determine
	the basic block.  Assert the use reference is not artificial and that
	it has an associated insn.

gcc/testsuite/
	PR rtl-optimization/84878
	* gcc.dg/pr84878.c: New test.

Index: gcc/ddg.c
===================================================================
--- gcc/ddg.c	(revision 258802)
+++ gcc/ddg.c	(working copy)
@@ -295,11 +295,14 @@ add_cross_iteration_register_deps (ddg_p
   /* Create inter-loop true dependences and anti dependences.  */
   for (r_use = DF_REF_CHAIN (last_def); r_use != NULL; r_use = r_use->next)
     {
-      rtx_insn *use_insn = DF_REF_INSN (r_use->ref);
-
-      if (BLOCK_FOR_INSN (use_insn) != g->bb)
+      if (DF_REF_BB (r_use->ref) != g->bb)
 	continue;
 
+      gcc_assert (!DF_REF_IS_ARTIFICIAL (r_use->ref)
+		  && DF_REF_INSN_INFO (r_use->ref) != NULL);
+
+      rtx_insn *use_insn = DF_REF_INSN (r_use->ref);
+
       /* ??? Do not handle uses with DF_REF_IN_NOTE notes.  */
       use_node = get_node_of_insn (g, use_insn);
       gcc_assert (use_node);
Index: gcc/testsuite/gcc.target/powerpc/pr84878.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr84878.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr84878.c	(working copy)
@@ -0,0 +1,18 @@
+/* PR target/84878 */
+/* { dg-do compile } */
+/* { dg-require-effective-target powerpc_altivec_ok } */
+/* { dg-options "-O2 -maltivec -mno-vsx -fmodulo-sched -ftree-vectorize -funroll-loops -fassociative-math -fno-signed-zeros -fno-trapping-math" } */
+
+int ek;
+float zu;
+
+int
+k5 (int ks)
+{
+  while (ek < 1)
+    {
+      ks += (int)(0x1000000 + zu + !ek);
+      ++ek;
+    }
+  return ks;
+}