Re: [tree-ssa] Get rid of LOOP_EXPRs

[law at redhat dot com]

In message <20030825173002.GC15168@atrey.karlin.mff.cuni.cz>, Zdenek Dvorak wri
tes:
 >Hello,
 >
 >> The patch to remove LOOP_EXPRs is currently blocked on a couple 
 >> regressions that you still need to fix.
 >> 
 >>   1. 920929-1.c fails without the change to reset DECL_TOO_LATE.  In
 >>      a separate thread both Jason and I asked you for more details
 >>      about the integrate.c change which resets DECL_TOO_LATE.
 >> 
 >>      My main concern is that you could lose valid error messages 
 >>      in inlined functions if you reset this bit.  Consider the case
 >>      if we end up threading a jump from outside the contour to the
 >>      too-late label inside the contour.
 >
 >I don't see this -- this bit is here exactly to get these messages and
 >it is set during expansion accordingly.  The patch just fixes the
 >problem when it is not zeroed before expansion as expected.
920929-1.c certainly fails with -O3 without the DECL_TOO_LATE patch.  And
while the DECL_TOO_LATE patch is clearly safe for that example, I've been
unable to convince myself that it is safe in general.

Again, consider the case were we have an saved function tree where we
have set DECL_TOO_LATE on various labels to indicate we can't jump from
outside those contexts to inside those contexts.  Now assume that we
inline that saved function tree into some other function and that 
we're able to thread some jump from outside the inlined function's context
to a label with DECL_TOO_LATE set.  That should be an error (and the 
jump threading code should avoid the situation).  However with your 
DECL_TOO_LATE patch, it seems to me that such a goof by the jump threading
code would be silently ignored, likely leading to incorrect code.

 >>   2. I get regressions for gcc.misc-tests/gcov-10.c when I use your
 >>      LOOP_EXPR removal patch.  In a nutshell we have a line which
 >>      is supposed to be executed 11 times, yet the gcov data shows
 >>      is as being executed only 6 times.
 >> 
 >> Both of these issues must be addressed before we can integrate your
 >> LOOP_EXPR removal patch.
 >
 >I have looked at that and it seemed to me that it is just a matter of
 >definition what is correct result (i.e. if I should decide what result
 >is more correct, I could not because both are from some point of view);
 >the test looks like
 >
 >for (i = 0; i < 10; i++) if (odd (i)) something;
 >
 >Whether the result should be the number of executions of if or of
 >something seems somewhat arbitrary to me.  I will recheck (now when
 >thinking about it, the number 6 is weird anyway).
Err, no, it's not that arbitrary.  Consider the controlling condition for
the for loop.   Barring loop restructuring, that test should execute
11 times (note the test is compiled *without* optimization).  The test
is verifying that gcov's view of this loop matches reality -- ie, if gcov
thinks the test was executed > 11 or < 11 times, then something is
wrong and needs to be fixed.

Jeff