Bug 24626 - [4.1 Regression] internal compiler error: verify_flow_info failed
Summary: [4.1 Regression] internal compiler error: verify_flow_info failed
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 4.1.0
: P1 critical
Target Milestone: 4.1.0
Assignee: Richard Biener
URL: http://gcc.gnu.org/ml/gcc-patches/200...
Keywords: ice-on-valid-code, patch
Depends on: 10024
Blocks:
  Show dependency treegraph
 
Reported: 2005-11-01 23:00 UTC by Richard Biener
Modified: 2006-01-26 16:45 UTC (History)
8 users (show)

See Also:
Host:
Target: hppa*-*-* (32bit only)
Build:
Known to work: 3.3.3 4.2.0
Known to fail: 4.1.0
Last reconfirmed: 2005-11-17 11:42:48


Attachments
reduced testcase (587 bytes, text/plain)
2005-11-01 23:01 UTC, Richard Biener
Details
Patch (259 bytes, patch)
2005-12-10 22:38 UTC, John David Anglin
Details | Diff
dump files for the ppc ICE (7.48 KB, application/octet-stream)
2006-01-13 21:55 UTC, Richard Biener
Details
patch (1.63 KB, patch)
2006-01-16 11:24 UTC, Richard Biener
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Biener 2005-11-01 23:00:27 UTC
We fail to compile a number of packages like

/usr/lib/gcc/hppa-suse-linux/4.1.0/cc1 -fpreprocessed db.i -quiet -dumpbase db.c -auxbase-strip .libs/db.o -O2 -Wall -version -fmessage-length=0 -fno-strict-aliasing -fPIC -o db.s

../dist/../db/db.c:385: error: wrong amount of branch edges after conditional jump 25      ../dist/../db/db.c:385: internal compiler error: verify_flow_info failed                   Please submit a full bug report,                                                           with preprocessed source if appropriate.                                                   See <URL:http://www.suse.de/feedback> for instructions.
Comment 1 Richard Biener 2005-11-01 23:01:03 UTC
Created attachment 10109 [details]
reduced testcase

testcase
Comment 2 b.gunreben 2005-11-09 07:07:12 UTC
Another testcase, fails with gcc -O2 -c, same compiler as above:

typedef long
(*bla)(int *node);

static long F2(void *tree, long blk, bla after_node_func)
{
 long call_result = 0;
 int *node;


 if (call_result = after_node_func(node))
  goto error_free_node;

 T(node);
 return 0;

error_free_node:
 T(node);
error:
 return call_result;
}

long F1(void *tree)
{
 return F2(tree, F3(tree), (void *)0);
}
Comment 3 Richard Biener 2005-11-12 23:03:47 UTC
works with 3.3, so a regression.
Comment 4 Janis Johnson 2005-11-17 00:02:07 UTC
A regression hunt with an hppa-linux cross compiler on powerpc-linux using
the testcase from comment #2 identified this large RTL loop versioning
patch:

http://gcc.gnu.org/viewcvs?view=rev&rev=97481

r97481 | hagog | 2005-04-03 08:44:33 +0000 (Sun, 03 Apr 2005) | 29 lines
Comment 5 Richard Biener 2005-11-17 11:42:48 UTC
So I guess being able to hunt it confirms it.
Comment 6 Andrew Pinski 2005-11-19 01:16:08 UTC
Testing this on a cross to hppa2.0w-hp-hpux11.11 to see if this fails there so it can be considered exposed on a primary target.
Comment 7 Andrew Pinski 2005-11-19 01:27:04 UTC
Confirmed on hppa2.0w-hp-hpux11.11 also which means this effects a primary target also.
Comment 8 Mark Mitchell 2005-11-19 01:37:54 UTC
Showstopper.
Comment 9 Andrew Pinski 2005-11-19 02:11:17 UTC
HUH:
;; succ:       2 [100.0%]  (fallthru)

(jump_insn 21 19 31 0 (parallel [
            (set (pc)
                (if_then_else (eq (reg:SI 28 %r28)
                        (const_int 0 [0x0]))
                    (label_ref 31)
                    (pc)))
            (set (reg/v:SI 3 %r3 [orig:94 call_result ] [94])
                (reg:SI 28 %r28))
        ]) 225 {*pa.md:8683} (insn_list:REG_DEP_TRUE 19 (nil))
    (expr_list:REG_DEAD (reg:SI 28 %r28)
        (expr_list:REG_BR_PROB (const_int 10000 [0x2710])
            (nil))))


The edges are incorrect for some reason.
Comment 10 Andrew Pinski 2005-11-19 02:31:12 UTC
Here is a shorter testcase:
long fff(int*);

long F2(int *node)
{
 long call_result = 0;

 if (call_result = fff(node))
  goto error_free_node;

 T(node);
 return 0;

error_free_node:
 T(node);
 return call_result;
}
-------

The problem is related to that instruction pattern in comment #9, for some reason we think it is just a jump so we remove the fall through edge and BB and then we ICE as we have a conditional jump and only one edge.

any_condjump_p returns the correct value of one but cleanup edges must use a different function to check for this.  Looking more into this.
Comment 11 Andrew Pinski 2005-11-19 02:37:53 UTC
Hmm:
Simplifying condjump 15 around jump 67
Merged 2 and 3 without moving.


But the edge is the same now :)
as:
Cross jumping from bb 1 to bb 2; 2 common insns
Deleted label in block 3.

But we don't delete the conditional jump as it sets other stuf besides the PC.
Comment 12 Andrew Pinski 2005-11-19 02:42:27 UTC
Hmm:
what changed is:
       if (n_branch != 1 && any_condjump_p (BB_END (bb))
-	  && JUMP_LABEL (BB_END (bb)) != BB_HEAD (fallthru->dest))
+	  && JUMP_LABEL (BB_END (bb)) == BB_HEAD (fallthru->dest))


So the verify is what was changed.  Looking some more.
Comment 13 dave 2005-11-19 02:44:23 UTC
Subject: Re:  [4.1 Regression] internal compiler error: verify_flow_info failed

> But the edge is the same now :)
> as:
> Cross jumping from bb 1 to bb 2; 2 common insns
> Deleted label in block 3.
> 
> But we don't delete the conditional jump as it sets other stuf besides the PC.

This sounds familiar ;(

Dave
Comment 14 Andrew Pinski 2005-11-19 02:52:41 UTC
(In reply to comment #13)
> This sounds familiar ;(
It should because it was:
http://gcc.gnu.org/ml/gcc-patches/2003-06/msg02416.html
Comment 15 Andrew Pinski 2005-11-19 02:55:57 UTC
This is related to the old PR 10024 which has the same instruction.
Comment 16 Andrew Pinski 2005-11-19 03:06:19 UTC
(In reply to comment #12)
> Hmm:
> what changed is:
>        if (n_branch != 1 && any_condjump_p (BB_END (bb))
> -         && JUMP_LABEL (BB_END (bb)) != BB_HEAD (fallthru->dest))
> +         && JUMP_LABEL (BB_END (bb)) == BB_HEAD (fallthru->dest))
> So the verify is what was changed.  Looking some more.
Actually I don't see why this change was done, it looks wrong as if you have only a fallthru edge, and you have a conditional jump, that edge better be also the jump label too.

David or Richard, could you test the revert of this part of the patch, it should fix the testcase.
Comment 17 dave 2005-11-19 03:41:43 UTC
Subject: Re:  [4.1 Regression] internal compiler error: verify_flow_info failed

> >        if (n_branch != 1 && any_condjump_p (BB_END (bb))
> > -         && JUMP_LABEL (BB_END (bb)) != BB_HEAD (fallthru->dest))
> > +         && JUMP_LABEL (BB_END (bb)) == BB_HEAD (fallthru->dest))
> > So the verify is what was changed.  Looking some more.
> Actually I don't see why this change was done, it looks wrong as if you have
> only a fallthru edge, and you have a conditional jump, that edge better be also
> the jump label too.
> 
> David or Richard, could you test the revert of this part of the patch, it
> should fix the testcase.

I tried both short testcases and they assembled without problem at -O2
with and without -fno-strict-aliasing -fPIC.  Full build and check in
progress.

Dave
Comment 18 John David Anglin 2005-12-10 22:38:58 UTC
Created attachment 10449 [details]
Patch
Comment 19 John David Anglin 2005-12-10 22:41:04 UTC
Andrew, I had your patch in my tree for some time and haven't seen any
adverse effects.  Would you like to submit or install it as obvious?
Comment 20 Andrew Pinski 2005-12-10 23:02:07 UTC
(In reply to comment #19)
> Andrew, I had your patch in my tree for some time and haven't seen any
> adverse effects.  Would you like to submit or install it as obvious?

I will deal with it soon as I download the SVN version of gcc on my laptop (which just started to work again).
Comment 21 Andrew Pinski 2005-12-12 02:38:34 UTC
Patch posted:
http://gcc.gnu.org/ml/gcc-patches/2005-12/msg00832.html

Comment 22 Steven Bosscher 2006-01-05 22:54:38 UTC
Mustafa's change (http://gcc.gnu.org/viewcvs?view=rev&rev=97481) has the following ChangeLog entry:

        (rtl_verify_flow_info_1): Fix.

@@ -2028,7 +2028,7 @@
          err = 1;
        }
       if (n_branch != 1 && any_condjump_p (BB_END (bb))
-         && JUMP_LABEL (BB_END (bb)) != BB_HEAD (fallthru->dest))
+         && JUMP_LABEL (BB_END (bb)) == BB_HEAD (fallthru->dest))
        {
          error ("Wrong amount of branch edges after conditional jump %i", bb->index);
          err = 1;


Makes me wonder what this was supposed to "fix".
Comment 23 Andrew Pinski 2006-01-08 05:18:39 UTC
I am no longer working on this bug, from what I hear it is really just to change the check "n_branch != 1" to "n_branch > 2" but I don't know if that is true or not.
Comment 24 Vladimir Yanovsky 2006-01-09 16:15:13 UTC
You might be right; Compiling the code below without Mustafa's patch to cfgrtl.c we get the following ICE on Power5: powerpc-linux if SMS is enabled:

gcc -O  -c -fmodulo-sched  smaller_comb.c

smaller_comb.c: In function foo:
smaller_comb.c:9: error: wrong amount of branch edges after conditional jump 9
smaller_comb.c:9: internal compiler error: verify_flow_info failed

this is the testcase:
int foo(short* vec1, short* vec2, short* vec3,int len )
{
        int temp,i;
        for (i=0; i<len; i++) {
                 temp = vec1[i] * 2;
                 temp += vec2[i] * 3 ;
                 vec3[i] = temp;
        }
}

n_branch is 0 at the time the error occurs.



(In reply to comment #23)
> I am no longer working on this bug, from what I hear it is really just to
> change the check "n_branch != 1" to "n_branch > 2" but I don't know if that is
> true or not.
> 

Comment 25 Steven Bosscher 2006-01-12 22:56:52 UTC
I can not reproduce this ICE.  We really need some way to reproduce the problem that Mustafa's patch was supposed to fix.

I use a cross-compiler from AMD64 to ppc:
GNU C version 4.2.0 20060112 (experimental) (powerpc64-unknown-linux-gnu)

And compiling the test case from comment #24 does not reproduce the ICE for me.  It just compiles without problems.  I used the following command line:
$ ./cc1 -O -mcpu=power5 -fmodulo-sched smaller_comb.c
Comment 26 dave 2006-01-13 00:12:11 UTC
Subject: Re:  [4.1/4.2 Regression] internal compiler error: verify_flow_info failed

> ------- Comment #25 from steven at gcc dot gnu dot org  2006-01-12 22:56 -------
> I can not reproduce this ICE.  We really need some way to reproduce the problem
> that Mustafa's patch was supposed to fix.

This is still present in the trunk as of today.  I can reproduce it using
the testcase in comment #2 on hppa-unknown-linux-gnu.  This is the jump_insn:

(gdb) p debug_rtx (insn)
(jump_insn 21 19 31 2 (parallel [
            (set (pc)
		(if_then_else (eq (reg:SI 28 %r28)
			(const_int 0 [0x0]))
		    (label_ref 31)
		    (pc)))
	    (set (reg/v:SI 3 %r3 [orig:94 call_result ] [94])
		(reg:SI 28 %r28))
	]) 227 {*pa.md:8779} (insn_list:REG_DEP_TRUE 19 (nil))
    (expr_list:REG_DEAD (reg:SI 28 %r28)
	(expr_list:REG_BR_PROB (const_int 10000 [0x2710])
	    (nil))))

n_branch == 0.

Did you try the testcase in #24 "without" Mustafa's patch to cfgrtl.c?

Dave
Comment 27 Vladimir Yanovsky 2006-01-13 13:26:24 UTC
I've checked compiling the code in comment 24 with -mcpu=power5, compiled without problems. Compiling with -mcpu=power4  (or without -mcpu flag) did produce the ICE as can be seen below. 

gcc was configured as follows: ../gcc/configure   --enable-threads=posix --prefix=/Develop/mainline-yanov/install/ --enable-shared --enable-__cxa_atexit --host=powerpc-suse-linux --build=powerpc-suse-linux --target=powerpc-suse-linux --enable-targets=powerpc64-suse-linux --enable-biarch --enable-checking --enable-libgcj --with-system-zlib --enable-languages=c,c++,fortran


yanov@rivka:~/smstests/sms_tests> /Develop/mainline-yanov/install/bin/gcc -fmod ulo-sched -c -O  smaller_comb.c
smaller_comb.c: In function ‘foo’:
smaller_comb.c:9: error: wrong amount of branch edges after conditional jump 9,  n_branch=0
smaller_comb.c:9: internal compiler error: verify_flow_info failed
Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://gcc.gnu.org/bugs.html> for instructions.
yanov@rivka:~/smstests/sms_tests>
yanov@rivka:~/smstests/sms_tests>
yanov@rivka:~/smstests/sms_tests> /Develop/mainline-yanov/install/bin/gcc -fmod ulo-sched -c -O -mcpu=power5 smaller_comb.c
yanov@rivka:~/smstests/sms_tests>
yanov@rivka:~/smstests/sms_tests>
yanov@rivka:~/smstests/sms_tests> /Develop/mainline-yanov/install/bin/gcc -fmod ulo-sched -c -O -mcpu=power4 smaller_comb.c
smaller_comb.c: In function ‘foo’:
smaller_comb.c:9: error: wrong amount of branch edges after conditional jump 9,  n_branch=0
smaller_comb.c:9: internal compiler error: verify_flow_info failed
Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://gcc.gnu.org/bugs.html> for instructions.

Vladimir











(In reply to comment #26)
> Subject: Re:  [4.1/4.2 Regression] internal compiler error: verify_flow_info
> failed
> 
> > ------- Comment #25 from steven at gcc dot gnu dot org  2006-01-12 22:56 -------
> > I can not reproduce this ICE.  We really need some way to reproduce the problem
> > that Mustafa's patch was supposed to fix.
> 
> This is still present in the trunk as of today.  I can reproduce it using
> the testcase in comment #2 on hppa-unknown-linux-gnu.  This is the jump_insn:
> 
> (gdb) p debug_rtx (insn)
> (jump_insn 21 19 31 2 (parallel [
>             (set (pc)
>                 (if_then_else (eq (reg:SI 28 %r28)
>                         (const_int 0 [0x0]))
>                     (label_ref 31)
>                     (pc)))
>             (set (reg/v:SI 3 %r3 [orig:94 call_result ] [94])
>                 (reg:SI 28 %r28))
>         ]) 227 {*pa.md:8779} (insn_list:REG_DEP_TRUE 19 (nil))
>     (expr_list:REG_DEAD (reg:SI 28 %r28)
>         (expr_list:REG_BR_PROB (const_int 10000 [0x2710])
>             (nil))))
> 
> n_branch == 0.
> 
> Did you try the testcase in #24 "without" Mustafa's patch to cfgrtl.c?
> 
> Dave
> 

Comment 28 Steven Bosscher 2006-01-13 17:48:00 UTC
I am unable to reproduce the PowerPC SMS ICE with an AMD64 x PowerPC compiler.
Comment 29 Steven Bosscher 2006-01-13 17:56:46 UTC
Re. comment #26, Dave: I think that Mustafa's patch is actually correct, but I'm not sure.  I _can_ reproduce the HPPA problem (at least, I used to be able to reproduce it when I still had access to a HPPA box), but not the PowerPC SMS ICE, indeed with Mustafa's patch reverted.  I'm going to spend a few more hours on this.  But I don't have access to HPPA or PowerPC so there is really not much I can do other than trying to reproduce the ICEs with cross-compilers and figure out what is wrong.
Comment 30 Steven Bosscher 2006-01-13 18:51:22 UTC
I wonder if HP-PA should not just make targetm.cannot_modify_jumps_p return true after reload...
Comment 31 Steven Bosscher 2006-01-13 19:24:24 UTC
First, we need the following patch to try_simplify_condjump.

-----------------------------------------------------------------------------
Index: cfgcleanup.c
===================================================================
--- cfgcleanup.c        (revision 109649)
+++ cfgcleanup.c        (working copy)
@@ -114,9 +114,10 @@ try_simplify_condjump (basic_block cbran
     return false;

   /* Verify that we've got a normal conditional branch at the end
-     of the block.  */
+     of the block.  Some targets, e.g. HP-PA, have insns that do a
+     jump and a set in one.  We can't handle that here.  */
   cbranch_insn = BB_END (cbranch_block);
-  if (!any_condjump_p (cbranch_insn))
+  if (!any_condjump_p (cbranch_insn) || !onlyjump_p (cbranch_insn))
     return false;

   cbranch_fallthru_edge = FALLTHRU_EDGE (cbranch_block);
-----------------------------------------------------------------------------

Without the patch. try_simplify_condjump messes up the CFG because it only tests any_condjump_p on the jump instruction, but not onlyjump_p.  This is what happens on mainline.

But unfortunately we only move the problem elsewhere with that patch.

At the point of failure we have the following CFG:

     ENTRY
       |
       2
       |\
       | 3
       |/
       4
       |
       5
       |
     EXIT

with basic block 2 ending in the !onlyjump_p jump:
(jump_insn 18 16 21 2 (parallel [
            (set (pc)
                (if_then_else (ne (reg:SI 28 %r28)
                        (const_int 0 [0x0]))
                    (label_ref 29)
                    (pc)))
            (set (reg/v:SI 3 %r3 [orig:95 call_result ] [95])
                (reg:SI 28 %r28))
        ]) 227 {*pa.md:8779} (insn_list:REG_DEP_TRUE 16 (nil))
    (expr_list:REG_DEAD (reg:SI 28 %r28)
        (expr_list:REG_BR_PROB (const_int 5000 [0x1388])
            (nil))))

and basic block 3 a trivial forwarder block:

;; basic block 3, loop depth 0, count 0
;; prev block 2, next block 4
;; pred:       2 [50.0%]  (fallthru)
;; succ:       4 [100.0%]  (fallthru)
;; Registers live at start:  3 [%r3] 30 [%r30] 48 [%fr12]
(note 21 18 29 3 [bb 3] NOTE_INSN_BASIC_BLOCK)
;; Registers live at end:  3 [%r3] 30 [%r30] 48 [%fr12]

So try_optimize_cfg tries to turn the CFG into straight-line code, i.e. it tries the following transformation:

     ENTRY           ENTRY
       |               |
       2               2
       |\              |
       | 3    ==>      |
       |/              |
       4               4
       |               |
       5               5
       |               |
     EXIT            EXIT

This transformation is done in the following code, with basic block 3 as B:

              /* If B has a single outgoing edge, but uses a
                 non-trivial jump instruction without side-effects, we
                 can either delete the jump entirely, or replace it
                 with a simple unconditional jump.  */
              if (single_succ_p (b)
                  && single_succ (b) != EXIT_BLOCK_PTR
                  && onlyjump_p (BB_END (b))
                  && !find_reg_note (BB_END (b), REG_CROSSING_JUMP, NULL_RTX)
                  && try_redirect_by_replacing_jump (single_succ_edge (b),
                                                     single_succ (b),
                                                     (mode & CLEANUP_CFGLAYOUT) != 0))
                {
                  update_forwarder_flag (b);
                  changed_here = true;
                }

So B, basic block 3, has only one successor, which is not the EXIT block, and it ends in an onlyjump_p jump.  try_redirect_by_replacing_jump has no trouble with this jump, it simply removes it.  Now we have:

     ENTRY
       |
       2
       |\
       | 3 (empty)
       |/
       4
       |
       5
       |
     EXIT

and we mark basic block 3 as (still) a forwarder block.  Then we iterate, and try to optimize basic block 3 again.  And we remove the whole block because it is empty now (NB: b is still basic block 3):

              /* If we fall through an empty block, we can remove it.  */
              if (!(mode & CLEANUP_CFGLAYOUT)
                  && single_pred_p (b)
                  && (single_pred_edge (b)->flags & EDGE_FALLTHRU)
                  && !LABEL_P (BB_HEAD (b))
                  && FORWARDER_BLOCK_P (b)
                  /* Note that forwarder_block_p true ensures that
                     there is a successor for this block.  */
                  && (single_succ_edge (b)->flags & EDGE_FALLTHRU)
                  && n_basic_blocks > NUM_FIXED_BLOCKS + 1)
                {
                  if (dump_file)
                    fprintf (dump_file,
                             "Deleting fallthru block %i.\n",
                             b->index);

                  c = b->prev_bb == ENTRY_BLOCK_PTR ? b->next_bb : b->prev_bb;
                  redirect_edge_succ_nodup (single_pred_edge (b),
                                            single_succ (b));
                  delete_basic_block (b);
                  changed = true;
                  b = c;
                }

So I plugged a little hole there:
-----------------------------------------------------------------------------
@@ -2039,6 +2040,9 @@ try_optimize_cfg (int mode)
              /* If we fall through an empty block, we can remove it.  */
              if (!(mode & CLEANUP_CFGLAYOUT)
                  && single_pred_p (b)
+                 && ((c = single_pred (b)) == ENTRY_BLOCK_PTR
+                     || !JUMP_P (BB_END (c))
+                     || onlyjump_p (BB_END (c)))
                  && (single_pred_edge (b)->flags & EDGE_FALLTHRU)
                  && !LABEL_P (BB_HEAD (b))
                  && FORWARDER_BLOCK_P (b)
-----------------------------------------------------------------------------

but that leaves us with the empty basic block 3, and we later on (not yet determined where) we die on it again in cfgcleanup.
Comment 32 dave 2006-01-13 19:36:22 UTC
Subject: Re:  [4.1/4.2 Regression] internal compiler error: verify_flow_info failed

> First, we need the following patch to try_simplify_condjump.

Excellent progress.

I was going to say that I can reproduce on x86 using hppa-unknown-linux-gnu
target with testcase #2.

Dave
Comment 33 Richard Biener 2006-01-13 21:55:12 UTC
Created attachment 10638 [details]
dump files for the ppc ICE

Attached dump files for ppc -O -fmodulo-sched -da.
gcc (GCC) 4.1.0 20051211 (prerelease) (SUSE Linux), which has mostafas patch reverted.
Comment 34 Richard Biener 2006-01-13 22:06:43 UTC
With gcc (GCC) 4.1.0 20060109 (prerelease) (SUSE Linux) I can no longer reproduce the PPC ICE.
Comment 35 Steven Bosscher 2006-01-13 22:09:48 UTC
After too much trial-and-error, and thinking about it some more, I think we should approach this as a SMS or powerpc bug.  The code that Mustafa changed makes us reject the RTL equivalent of "if (a) goto b; b: ...", i.e. a jump to the next instruction ;-)

My initial reaction was that you want to clean that jump up, but then again, the code Mustafa changed explicitly allowed this kind of jump.  We had:
 
  if (n_branch != 1 && any_condjump_p (BB_END (bb))
      && JUMP_LABEL (BB_END (bb)) != BB_HEAD (fallthru->dest))

which says that if you have a conditional jump, there must be at least one branch edge, _unless_ the jump is to the next instruction.  In the latter case, there is only one edge in the CFG from bb to single_succ(bb), which represents both the fallthru and the branch cases of the conditional jump.

The problem is that reverting Mustafa's "fix" exposes an ICE on PowerPC.  I think we should try to understand where that comes from before reverting.
Comment 36 Richard Biener 2006-01-13 22:29:53 UTC
But this ICE is fixed or papered over on both the 4.1 branch and trunk now.
Comment 37 Steven Bosscher 2006-01-13 23:26:19 UTC
I think I know what the problem is.  At the point where we error in cfgrtl, we have the following basic block:

Breakpoint 3, rtl_verify_flow_info_1 () at cfgrtl.c:2051
2051              error ("wrong amount of branch edges after conditional jump %i", bb->index);
(gdb) l
2046              err = 1;
2047            }
2048          if (n_branch != 1 && any_condjump_p (BB_END (bb))
2049              && JUMP_LABEL (BB_END (bb)) != BB_HEAD (fallthru->dest))
2050            {
2051              error ("wrong amount of branch edges after conditional jump %i", bb->index);
2052              err = 1;
2053            }
2054          if (n_call && !CALL_P (BB_END (bb)))
2055            {
(gdb) p debug_bb(bb)
;; basic block 7, loop depth 0, count 0
;; prev block 1, next block 9
;; pred:       1 [100.0%]  (fallthru)
;; succ:       9 [100.0%]  (fallthru) 8 (true)
;; Registers live at start:  1 [1] 31 [31] 67 [ap] 113 [sfp] 123 127 128 129 139
(note 89 68 90 7 [bb 7] NOTE_INSN_BASIC_BLOCK)
(insn 90 89 91 7 (set (reg:CC 140)
        (compare:CC (reg:SI 139)
            (const_int 1 [0x1]))) -1 (nil)
    (nil))
(jump_insn 91 90 94 7 (set (pc)
        (if_then_else (gt (reg:CC 140)
                (const_int 0 [0x0]))
            (label_ref:SI 93)
            (pc))) 472 {*rs6000.md:12875} (nil)
    (nil))
;; Registers live at end:  1 [1] 31 [31] 67 [ap] 113 [sfp] 123 127 128 129 139


Note the "(true)" flag on the edge to block 8.  That is the EDGE_TRUE_VALUE flag, which is not supposed to appear on trees.  We fail to compute n_branch properly because of this:

          if ((e->flags & ~(EDGE_DFS_BACK
                            | EDGE_CAN_FALLTHRU
                            | EDGE_IRREDUCIBLE_LOOP
                            | EDGE_LOOP_EXIT
                            | EDGE_CROSSING)) == 0)
            n_branch++;

Aha, but we have (e->flags & ~(EDGE_DFS_BACK
                            | EDGE_CAN_FALLTHRU
                            | EDGE_IRREDUCIBLE_LOOP
                            | EDGE_LOOP_EXIT
                            | EDGE_CROSSING)) == EDGE_TRUE_VALUE)
so n_branch stays at 0.  Then we go on checking with the wrong n_branch, and we get to the point where we fail.

My best bet: loop versioning still has bits that assume it works on trees only.


Comment 38 Steven Bosscher 2006-01-13 23:28:21 UTC
s/which is not supposed to appear on trees/which is not supposed to appear on RTL/

The EDGE_{TRUE,FALSE}_VALUE flags are for tree-ssa only.  On RTL they have no meaning.  We should probably add a check for this in rtl_verify_flow_info for this.
Comment 39 Steven Bosscher 2006-01-13 23:39:49 UTC
We should do the following:

Index: cfgloopmanip.c
===================================================================
--- cfgloopmanip.c      (revision 108361)
+++ cfgloopmanip.c      (working copy)
@@ -1419,7 +1419,7 @@ lv_adjust_loop_entry_edge (basic_block f
   lv_add_condition_to_bb (first_head, second_head, new_head,
                          cond_expr);

-  e1 = make_edge (new_head, first_head, EDGE_TRUE_VALUE);
+  e1 = make_edge (new_head, first_head, ir_type () ? EDGE_TRUE_VALUE : 0);
   set_immediate_dominator (CDI_DOMINATORS, first_head, new_head);
   set_immediate_dominator (CDI_DOMINATORS, second_head, new_head);

Index: cfgrtl.c
===================================================================
--- cfgrtl.c    (revision 108361)
+++ cfgrtl.c    (working copy)
@@ -2046,7 +2046,7 @@ rtl_verify_flow_info_1 (void)
          err = 1;
        }
       if (n_branch != 1 && any_condjump_p (BB_END (bb))
-         && JUMP_LABEL (BB_END (bb)) == BB_HEAD (fallthru->dest))
+         && JUMP_LABEL (BB_END (bb)) != BB_HEAD (fallthru->dest))
        {
          error ("wrong amount of branch edges after conditional jump %i", bb->index);
          err = 1;
  
Comment 40 dave 2006-01-14 00:57:21 UTC
Subject: Re:  [4.1/4.2 Regression] internal compiler error: verify_flow_info failed

> ------- Comment #30 from steven at gcc dot gnu dot org  2006-01-13 18:51 -------
> I wonder if HP-PA should not just make targetm.cannot_modify_jumps_p return
> true after reload...

I assume this question is somewhat independent of the PR.

I didn't think that this was necessary.  There is some somewhat horrible
code to save register r1 into the frame and restore it in the delay slot
of a jump in situations where we need a register for the jump (i.e.,
the jump is too large to be handled using a pc-relative offset).  The
downside is the jump needs several instructions more than would be needed
if we had a register handy.

Dave
Comment 41 Vladimir Yanovsky 2006-01-14 09:29:37 UTC
This fixes the problem on PowerPC.


(In reply to comment #39)
> We should do the following:
> 
> Index: cfgloopmanip.c
> ===================================================================
> --- cfgloopmanip.c      (revision 108361)
> +++ cfgloopmanip.c      (working copy)
> @@ -1419,7 +1419,7 @@ lv_adjust_loop_entry_edge (basic_block f
>    lv_add_condition_to_bb (first_head, second_head, new_head,
>                           cond_expr);
> 
> -  e1 = make_edge (new_head, first_head, EDGE_TRUE_VALUE);
> +  e1 = make_edge (new_head, first_head, ir_type () ? EDGE_TRUE_VALUE : 0);
>    set_immediate_dominator (CDI_DOMINATORS, first_head, new_head);
>    set_immediate_dominator (CDI_DOMINATORS, second_head, new_head);
> 
> Index: cfgrtl.c
> ===================================================================
> --- cfgrtl.c    (revision 108361)
> +++ cfgrtl.c    (working copy)
> @@ -2046,7 +2046,7 @@ rtl_verify_flow_info_1 (void)
>           err = 1;
>         }
>        if (n_branch != 1 && any_condjump_p (BB_END (bb))
> -         && JUMP_LABEL (BB_END (bb)) == BB_HEAD (fallthru->dest))
> +         && JUMP_LABEL (BB_END (bb)) != BB_HEAD (fallthru->dest))
>         {
>           error ("wrong amount of branch edges after conditional jump %i",
> bb->index);
>           err = 1;
> 
> 

Comment 42 John David Anglin 2006-01-14 17:58:35 UTC
The patch in #41 doesn't seem to be a complete fix.   I see the following
assembler code for testcase #2 on hppa-unknown-linux-gnu:

        .LEVEL 1.1
        .text
        .align 4
.globl F1
        .type   F1, @function
F1:
        .PROC
        .CALLINFO FRAME=64,CALLS,SAVE_RP,ENTRY_GR=3
        .ENTRY
        stw %r2,-20(%r30)
        bl F3,%r2
        stwm %r3,64(%r30)
        ldi 0,%r22
        copy %r3,%r26
        .CALL   ARGW0=GR
        bl $$dyncall,%r31
        copy %r31,%r2
        copy %r28,%r3
.L2:
        bl T,%r2
        copy %r3,%r26
        ldw -84(%r30),%r2
        copy %r3,%r28
        bv %r0(%r2)
        ldwm -64(%r30),%r3
        .EXIT
        .PROCEND
        .size   F1, .-F1
        .ident  "GCC: (GNU) 4.2.0 20060113 (experimental)"

We save register %r3, but it's not initialized.  
Comment 43 John David Anglin 2006-01-14 18:16:45 UTC
Doh, forget my last comment.  The lack of initialization just reflects
the testcase.
Comment 44 Steven Bosscher 2006-01-14 18:18:31 UTC
Dave, this is another comment that isn't helpful.  What is wrong in the assembler output for comment #42, what do you expect the assembler output to look like, how is the problem related to this bug (if it is), and how do I reproduce it?
Comment 45 dave 2006-01-14 18:59:25 UTC
Subject: Re:  [4.1/4.2 Regression] internal compiler error: verify_flow_info failed

> Dave, this is another comment that isn't helpful.  What is wrong in the
> assembler output for comment #42, what do you expect the assembler output to
> look like, how is the problem related to this bug (if it is), and how do I
> reproduce it?

Sorry for being vague.  The issue is the following code:

        stw %r2,-20(%r30)
	bl F3,%r2
	stwm %r3,64(%r30)
	ldi 0,%r22
	copy %r3,%r26

r3 is not an argument register in F1 and it is never initialized.  The
copy instruction copies r3 to r26 to initializing the first argument (node)
of the call to after_node_func.  Thus, if the call really depended on
the value, it would generate garbage.  The lack of initialization just
reflects the C code.  I guess if there's a bug here, it's the fact that
the compiler doesn't warn about the lack of initialization of node.

The lack of initialization leads to some strangeness later in the
assembly code:

        copy %r28,%r3
.L2:
	bl T,%r2
	copy %r3,%r26

In the first instruction, the result of the call to after_node_func
(call_result) is copied to register r3 and then used as the argument
for the call to T.  This seems wrong but I'm not sure since r3 wasn't
initialized in the first place.

Dave
Comment 46 dave 2006-01-14 19:28:06 UTC
Subject: Re:  [4.1/4.2 Regression] internal compiler error: verify_flow_info failed

> The lack of initialization leads to some strangeness later in the
> assembly code:
> 
>         copy %r28,%r3

I would have expected that the compiler copy r28 to a different register
(e.g., r4) here.  It suggests the register aliveness for r3 is messed up.

If I initialize the variable node, the compiler uses a different register.

I see in postreload:

(jump_insn 21 19 24 2 (parallel [
            (set (pc)
		(if_then_else (ne (reg:SI 28 %r28)
			(const_int 0 [0x0]))
		    (label_ref 31)
		    (pc)))
	    (set (reg/v:SI 3 %r3 [orig:94 call_result ] [94])
		(reg:SI 28 %r28))
	]) 227 {*pa.md:8779} (insn_list:REG_DEP_TRUE 19 (nil))
    (expr_list:REG_BR_PROB (const_int 5000 [0x1388])
	(nil)))
;; End of basic block 2, registers live:
 3 [%r3] 30 [%r30] 94

;; Start of basic block 3, registers live: 30 [%r30] 94
(note 24 21 26 3 [bb 3] NOTE_INSN_BASIC_BLOCK)

(insn 26 24 27 3 (set (reg:SI 26 %r26 [ node ])
	(reg/v/f:SI 3 %r3 [orig:95 node ] [95])) 37 {*pa.md:2308} (nil)
    (nil))

Note that insn 26 thinks r3 contains node but insn 21 has just set
it to call_result.

> .L2:
> 	bl T,%r2
> 	copy %r3,%r26

To generate the code, I simply compiled testcase #2 with gcc -O2 -S.
The compiler was built with the patch in comment #39.

Dave
Comment 47 dave 2006-01-14 19:48:14 UTC
Subject: Re:  [4.1/4.2 Regression] internal compiler error: verify_flow_info failed

> I see in postreload:
> 
> (jump_insn 21 19 24 2 (parallel [
>             (set (pc)
>                 (if_then_else (ne (reg:SI 28 %r28)
>                         (const_int 0 [0x0]))
>                     (label_ref 31)
>                     (pc)))
>             (set (reg/v:SI 3 %r3 [orig:94 call_result ] [94])
>                 (reg:SI 28 %r28))
>         ]) 227 {*pa.md:8779} (insn_list:REG_DEP_TRUE 19 (nil))
>     (expr_list:REG_BR_PROB (const_int 5000 [0x1388])
>         (nil)))
> ;; End of basic block 2, registers live:
>  3 [%r3] 30 [%r30] 94
> 
> ;; Start of basic block 3, registers live: 30 [%r30] 94
> (note 24 21 26 3 [bb 3] NOTE_INSN_BASIC_BLOCK)
> 
> (insn 26 24 27 3 (set (reg:SI 26 %r26 [ node ])
>         (reg/v/f:SI 3 %r3 [orig:95 node ] [95])) 37 {*pa.md:2308} (nil)
>     (nil))

In greg:

;; Start of basic block 2, registers live: 3 [%r3] 26 [%r26] 30 [%r30] 95
(note 9 7 20 2 [bb 2] NOTE_INSN_BASIC_BLOCK)


...

(jump_insn 21 19 24 2 (parallel [
            (set (pc)
		(if_then_else (ne (reg:SI 28 %r28)
			(const_int 0 [0x0]))
		    (label_ref 31)
		    (pc)))
	    (set (reg/v:SI 94 [ call_result ])
		(reg:SI 28 %r28))
	]) 227 {*pa.md:8779} (insn_list:REG_DEP_TRUE 19 (nil))
    (expr_list:REG_DEAD (reg:SI 28 %r28)
	(expr_list:REG_BR_PROB (const_int 5000 [0x1388])
    (nil))))
;; End of basic block 2, registers live:
 3 [%r3] 30 [%r30] 94 95

I don't see how reload can pick r3 for SI 94 given that r3 is alive
at the start and end of BB 2.

Dave
Comment 48 Steven Bosscher 2006-01-14 21:10:24 UTC
I certainly don't see any way how this new issue has anything to do with the ICE due to Mustafa's patch.

There's an easy check: is the code semantically equivalent to some other compiler you trust (e.g. older gcc, hp system compiler, anything)...
Comment 49 dave 2006-01-14 23:12:57 UTC
Subject: Re:  [4.1/4.2 Regression] internal compiler error: verify_flow_info failed

> ------- Comment #48 from steven at gcc dot gnu dot org  2006-01-14 21:10 -------
> I certainly don't see any way how this new issue has anything to do with the
> ICE due to Mustafa's patch.

Well, I doubt that you would ever see the ICE if reload hadn't used
r3 for two variables at the same time.  If you think this issue that
I've pointed out is a bug, I can file a new PR.

> There's an easy check: is the code semantically equivalent to some other
> compiler you trust (e.g. older gcc, hp system compiler, anything)...

This what you get with HP cc at +O2:

-bash-2.05b$ cc -Ae +O2 -S pr24626.c
cc: line 19: warning 5004: Uninitialized variable "node" in function "F2" (5004)

The assembly code is shown below.  The interesting part is that
node is initialized to 0 in the calls to after_node_func and T.
At +O3, HP cc doesn't generate the warning but it still generates
code that uses a consistent uninitialized value for node.

Although an uninitialized object may invoke undefined behavior,
I couldn't find anywhere in the C standard that suggests it's
legitimate to use the same storage for two objects in the same
block.  The HP code isn't equivalent to the GCC code as it uses
different storage for call_result and node.  Thus, the call to
T probably will invoke different behavior.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

	.LEVEL	2.0N

	.SPACE	$TEXT$,SORT=8
	.SUBSPA	$CODE$,QUAD=0,ALIGN=4,ACCESS=0x2c,CODE_ONLY,SORT=24
F2
	.PROC
	.CALLINFO CALLER,FRAME=16,SAVE_RP,ARGS_SAVED,ORDERING_AWARE
        .ENTRY
        STW     %r2,-20(%r30)   ;offset 0x0
        LDO     64(%r30),%r30   ;offset 0x4
        COPY    %r0,%r26        ;offset 0x8
        COPY    %r24,%r22       ;offset 0xc
        .CALL   ARGW0=GR,RTNVAL=GR      ;in=22,26;out=28;
        B,L     $$dyncall,%r31  ;offset 0x10
        COPY    %r31,%r2        ;offset 0x14
        CMPB,<> %r28,%r0,$00000002      ;offset 0x18
        COPY    %r28,%r31       ;offset 0x1c
$00000001
        .CALL   ARGW0=GR,RTNVAL=GR      ;in=26;out=28;
        B,L     T,%r2   ;offset 0x20
        COPY    %r0,%r26        ;offset 0x24
        MOVB,TR %r0,%r31,$D0    ;offset 0x28
        LDW     -84(%r30),%r2   ;offset 0x2c
$00000002
        STW     %r31,-64(%r30)  ;offset 0x30
        .CALL   ARGW0=GR,RTNVAL=GR      ;in=26;out=28;
        B,L     T,%r2   ;offset 0x34
        COPY    %r0,%r26        ;offset 0x38
        LDW     -64(%r30),%r31  ;offset 0x3c
$00000003
$L0
        LDW     -84(%r30),%r2   ;offset 0x40
$D0
        COPY    %r31,%r28       ;offset 0x44
        BVE     (%r2)   ;offset 0x48
        .EXIT
        LDO     -64(%r30),%r30  ;offset 0x4c
	.PROCEND	;in=24,25,26;out=28;



	.SPACE	$TEXT$
	.SUBSPA	$CODE$,QUAD=0,ALIGN=4,ACCESS=0x2c,CODE_ONLY,SORT=24
F1
	.PROC
	.CALLINFO CALLER,FRAME=16,SAVE_RP,ARGS_SAVED,ORDERING_AWARE
        .ENTRY
        STW     %r2,-20(%r30)   ;offset 0x50
        LDO     64(%r30),%r30   ;offset 0x54
        .CALL   ARGW0=GR,RTNVAL=GR      ;in=26;out=28;
        B,L     F3,%r2  ;offset 0x58
        STW     %r26,-64(%r30)  ;offset 0x5c
        LDW     -64(%r30),%r26  ;offset 0x60
        COPY    %r0,%r24        ;offset 0x64
        .CALL   ARGW0=GR,ARGW1=GR,ARGW2=GR,RTNVAL=GR    ;in=24,25,26;out=28;
        B,L     F2,%r2  ;offset 0x68
        COPY    %r28,%r25       ;offset 0x6c
        LDW     -84(%r30),%r2   ;offset 0x70
        BVE     (%r2)   ;offset 0x74
        .EXIT
        LDO     -64(%r30),%r30  ;offset 0x78
	.PROCEND	;in=26;out=28;



	.SPACE	$TEXT$
	.SUBSPA	$CODE$
	.SUBSPA	$CODE$
	.SPACE	$PRIVATE$,SORT=16
	.IMPORT	$$dyncall,MILLICODE
	.IMPORT	T,CODE
	.SPACE	$TEXT$
	.SUBSPA	$CODE$
	.EXPORT	F1,ENTRY,PRIV_LEV=3,ARGW0=GR,RTNVAL=GR,LONG_RETURN
	.IMPORT	F3,CODE
	.END
Comment 50 dave 2006-01-15 00:37:52 UTC
Subject: Re:  [4.1/4.2 Regression] internal compiler error: verify_flow_info failed

> There's an easy check: is the code semantically equivalent to some other
> compiler you trust (e.g. older gcc, hp system compiler, anything)...

I've enclosed the assembly code generated by gcc version 3.3.6
(Debian 1:3.3.6-10).  Although it's not as nicely optimized, it looks
correct to me.  The 3.4 code also looks ok.  However, the 4.0.3 is
wrong.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

	.LEVEL 1.1
	.text
	.align 4
	.type	F2, @function
F2:
	.PROC
	.CALLINFO FRAME=64,CALLS,SAVE_RP,ENTRY_GR=4
	.ENTRY
	stw %r2,-20(%r30)
	stwm %r4,64(%r30)
	stw %r3,-60(%r30)
	copy %r4,%r26
	copy %r24,%r22
	.CALL	ARGW0=GR
	bl $$dyncall,%r31
	copy %r31,%r2
	comib,<> 0,%r28,.L3
	copy %r28,%r3
	bl T,%r2
	copy %r4,%r26
	ldi 0,%r28
.L1:
	ldw -84(%r30),%r2
	ldw -60(%r30),%r3
	bv %r0(%r2)
	ldwm -64(%r30),%r4
.L3:
.L4:
	bl T,%r2
	copy %r4,%r26
	b .L1
	copy %r3,%r28
	.EXIT
	.PROCEND
	.size	F2, .-F2
	.align 4
.globl F1
	.type	F1, @function
F1:
	.PROC
	.CALLINFO FRAME=64,CALLS,SAVE_RP,ENTRY_GR=3
	.ENTRY
	stw %r2,-20(%r30)
	stwm %r4,64(%r30)
	bl F3,%r2
	copy %r26,%r4
	ldw -84(%r30),%r2
	copy %r28,%r25
	copy %r4,%r26
	ldi 0,%r24
	bl F2,%r0
	ldwm -64(%r30),%r4
	nop
	.EXIT
	.PROCEND
	.size	F1, .-F1
	.ident	"GCC: (GNU) 3.3.6 (Debian 1:3.3.6-10)"
Comment 51 Steven Bosscher 2006-01-15 00:47:08 UTC
*sigh*

Almost no-one knows HPPA assembly these days. It completely puzzles me why you call the assembler output "wrong" if you're not specifically stating what you're expecting instead.  But then, I don't think it is useful to look at test cases reduced to reproduce an ICE to the point where you have uninitialized variables.  There is no such thing as "wrong code" if you're using uninitialized variables.  At worst, you have found a missing diagnostic AFAICT.

I've tried hard enough to fix this bug and my patch from comment #39 is enough to fix the issue being discussed here.  I'm not going to look at this apparent new bug which can't possibly have anything to do with the problem that this PR was opened for.

Comment 52 dave 2006-01-15 02:37:05 UTC
Subject: Re:  [4.1/4.2 Regression] internal compiler error: verify_flow_info failed

> Almost no-one knows HPPA assembly these days. It completely puzzles me why you
> call the assembler output "wrong" if you're not specifically stating what
> you're expecting instead.  But then, I don't think it is useful to look at test

I did.  I said that I expected different registers to be used for
the variables "node" and "call_result".  However, with gcc 4.0 and
later, the variables are both placed in the same register "r3".  HP
cc, and gcc 3.3 and 3.4 use different registers.  The issue is
present in the RTL that I showed from the trunk.

I guess that I have stared at too much HPPA assembly.  I must admit
that x86 assembler is greek to me.

> cases reduced to reproduce an ICE to the point where you have uninitialized
> variables.  There is no such thing as "wrong code" if you're using
> uninitialized variables.  At worst, you have found a missing diagnostic AFAICT.

I don't agree.  Note that the function "F2" doesn't directly use the
value of "node".  It just passes its value to "after_node_func" and "T".
There is no information as to what these functions do with the value.
I could easily write implementations of "after_node_func" and "T"
that would cause "F2" to behaive differently between gcc 3.4 and 4.0
because the return from "after_node_func" changes the value of "node"
in "F2".  I can't do that if different storage is assigned to "node"
and "call_result".

Further, an uninitialized variable doesn't imply that the code can't
be wrong.  If you look at the documentation, you will see that the
warning is optional because GCC doesn't have enough information
to determine when the code might be correct despite appearing to
have an error.  I have to wonder about the no warning default in this
case as the lack of initialization is unambiguous.

A search of the C99 standard for "uninitialized" turned up no hits.
Thus, I would say GCC isn't allowed to assume the value of "node"
doesn't matter, and use it's storage for other purposes.

> I've tried hard enough to fix this bug and my patch from comment #39 is enough
> to fix the issue being discussed here.  I'm not going to look at this apparent
> new bug which can't possibly have anything to do with the problem that this PR
> was opened for.

Effort appreciated and I'm sure your patch fixes the ICE.  However, 
I also believe that gcc 4.0 and later generate wrong code for the
submitted testcase.  It's true that the submitter only complained
about the ICE, but I have to think that he also cares about the resulting
code ;)

Dave
Comment 53 Steven Bosscher 2006-01-15 04:05:30 UTC
FWIW, the part of the C99 standard that talk about uninitialized automatic variables are 3.17.2, 6.2.4, 6.7.8 (especially point 10), and 6.8.

Also see Appendix J2 "Undefined behavior".

To me, undefined behavior means that GCC could produce code to grow vegetables on the south pole or to shoot bunnies to the moon, and still not be wrong ;-)  So it can certainly do whatever it pleases with the uninitialized variable 'node' in the test case...
Comment 54 dave 2006-01-15 06:46:15 UTC
Subject: Re:  [4.1/4.2 Regression] internal compiler error: verify_flow_info failed

> FWIW, the part of the C99 standard that talk about uninitialized automatic
> variables are 3.17.2, 6.2.4, 6.7.8 (especially point 10), and 6.8.
> 
> Also see Appendix J2 "Undefined behavior".
> 
> To me, undefined behavior means that GCC could produce code to grow vegetables
> on the south pole or to shoot bunnies to the moon, and still not be wrong ;-) 
> So it can certainly do whatever it pleases with the uninitialized variable
> 'node' in the test case...

I can't see where the value of "node" is "used" other than as
arguments to function calls.  The value of "node" is indeterminate
but it's unclear to me from J2 whether this is sufficient to force
undefined behavior for F2.  HP cc and GCC versions prior to 4.0 
don't use the same storage for "call_result" and "node" in spite of
the fact that "node" is uninitialized.  As a result, we have a
portability issue between GCC 3 and 4.

J2 is informative.  Thus, the leap from indeterminate to undefined
isn't obvious.

Dave
Comment 55 Steven Bosscher 2006-01-15 09:22:27 UTC
The "leap" indeterminate to uninitialized is simply C99 3.4.3:

1. undefined behavior
behavior, upon use of a nonportable or erroneous program construct or of
erroneous data, for which this International Standard imposes no requirements
Comment 56 Richard Biener 2006-01-16 10:37:26 UTC
Steven, are your patches in comment #31 orthogonal to the fix proposed in comment #39, or is it strictly needed?
Comment 57 Richard Biener 2006-01-16 11:24:58 UTC
Created attachment 10652 [details]
patch

Testing the attached patch on x86-64 (all languages) and hppa (only C, my machine is slow).
Comment 58 Steven Bosscher 2006-01-16 21:52:43 UTC
Re. comment #56,
Richard, only the patch of comment #39 is necessary to fix this bug.

Comment 59 dave 2006-01-16 22:34:34 UTC
Subject: Re:  [4.1/4.2 Regression] internal compiler error: verify_flow_info failed

> ------- Comment #58 from steven at gcc dot gnu dot org  2006-01-16 21:52 -------
> Re. comment #56,
> Richard, only the patch of comment #39 is necessary to fix this bug.

I've tested the above change on hppa2.0w-hp-hpux11.11 and
hppa-unknown-linux-gnu with the trunk, and on hppa2.0w-hp-hpux11.11
with 4.1.0.  No regressions were seen.

Dave
Comment 60 Steven Bosscher 2006-01-20 00:06:20 UTC
I've tested it on x86_64-unknown-linux-gnu and mips-elf (simulator, C only).

Richi, I would have commited the patch for this myself as obvious, with a comment explaining why we have to call ir_type there and a promise to follow up with a patch for rtl_verify_flow_info to explicitly check for every edge that the EDGE_{TRUE,FALSE}_VALUE flags are not set.

I suggest you now commit the patch to mainline at least and maybe wait for Mark to give a verdict for GCC 4.1.  I'll craft the patch to do the extra checking some time next week.

Comment 61 Richard Biener 2006-01-20 09:39:01 UTC
Subject: Bug 24626

Author: rguenth
Date: Fri Jan 20 09:38:56 2006
New Revision: 110020

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=110020
Log:
2006-01-20  Richard Guenther  <rguenther@suse.de>
	Steven Bosscher <stevenb.gcc@gmail.com>

	PR rtl-optimization/24626
	* cfgloopmanip.c (lv_adjust_loop_entry_edge): Don't set
	EDGE_TRUE_VALUE if in RTL mode.

	Revert
	2005-03-30 Mostafa Hagog <mustafa@il.ibm.com>
	* cfgrtl.c (rtl_verify_flow_info_1): Fix.

	* gcc.dg/torture/pr24626-1.c: New testcase.
	* gcc.dg/torture/pr24626-2.c: Likewise.
	* gcc.dg/torture/pr24626-3.c: Likewise.
	* gcc.dg/torture/pr24626-4.c: Likewise.

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr24626-1.c
    trunk/gcc/testsuite/gcc.dg/torture/pr24626-2.c
    trunk/gcc/testsuite/gcc.dg/torture/pr24626-3.c
    trunk/gcc/testsuite/gcc.dg/torture/pr24626-4.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cfgloopmanip.c
    trunk/gcc/cfgrtl.c
    trunk/gcc/testsuite/ChangeLog

Comment 62 Richard Biener 2006-01-20 09:44:13 UTC
Done.  Let the flames come down to the two of us. :/  (I agree on the "obviousness" of the patch, but the part reverting Mostafas "fix" and four
testcases made the patch somewhat big).

So, fixed on mainline.
Comment 63 Steven Bosscher 2006-01-20 12:05:09 UTC
I still think there should be a comment in cfgloopmanip explaining the use of ir_type there, but I'll take care of that with the additional-checking patch.
Comment 64 Richard Biener 2006-01-23 10:01:41 UTC
Subject: Bug 24626

Author: rguenth
Date: Mon Jan 23 10:01:36 2006
New Revision: 110112

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=110112
Log:
2006-01-23  Richard Guenther  <rguenther@suse.de>
	Steven Bosscher <stevenb.gcc@gmail.com>

	PR rtl-optimization/24626
	* cfgloopmanip.c (lv_adjust_loop_entry_edge): Don't set
	EDGE_TRUE_VALUE if in RTL mode.

	Revert
	2005-03-30 Mostafa Hagog <mustafa@il.ibm.com>
	* cfgrtl.c (rtl_verify_flow_info_1): Fix.

	* gcc.dg/torture/pr24626-1.c: New testcase.
	* gcc.dg/torture/pr24626-2.c: Likewise.
	* gcc.dg/torture/pr24626-3.c: Likewise.
	* gcc.dg/torture/pr24626-4.c: Likewise.

Added:
    branches/gcc-4_1-branch/gcc/testsuite/gcc.dg/torture/pr24626-1.c
    branches/gcc-4_1-branch/gcc/testsuite/gcc.dg/torture/pr24626-2.c
    branches/gcc-4_1-branch/gcc/testsuite/gcc.dg/torture/pr24626-3.c
    branches/gcc-4_1-branch/gcc/testsuite/gcc.dg/torture/pr24626-4.c
Modified:
    branches/gcc-4_1-branch/gcc/ChangeLog
    branches/gcc-4_1-branch/gcc/cfgloopmanip.c
    branches/gcc-4_1-branch/gcc/cfgrtl.c
    branches/gcc-4_1-branch/gcc/testsuite/ChangeLog

Comment 65 Richard Biener 2006-01-23 10:06:46 UTC
Fixed.
Comment 66 Vladimir Yanovsky 2006-01-26 16:45:11 UTC
Bootstrapped and tested on PowerPC linux. No regressions.