[Bug fortran/57904] New: Bogus(?) "invokes undefined behavior" warning with Fortran's finalization wrapper (gfortran.dg/class_48.f90)

[Bug fortran/57904] New: Bogus(?) "invokes undefined behavior" warning with Fortran's finalization wrapper (gfortran.dg/class_48.f90)

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57904

            Bug ID: 57904
           Summary: Bogus(?) "invokes undefined behavior" warning with
                    Fortran's finalization wrapper
                    (gfortran.dg/class_48.f90)
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: burnus at gcc dot gnu.org

The test case is based on gfortran.dg/class_48.f90 - and the issue is exposed
by r200954.

Compiling the following test case with "gfortran -m32 -O2"  (or "-m32 -Os")
gives the warning:

test.f90: In function '__final_test2_T.1838.constprop.0':
test.f90:21:0: warning: iteration 2147483648 invokes undefined behavior
[-Waggressive-loop-optimizations]
       class(t), allocatable :: a
 ^


Test case:

program test
  call test2 ()
contains
  subroutine test2 ()
    type t
      integer, allocatable :: x
    end type t

    type t2
      class(t), allocatable :: a
    end type t2

    type(t2) :: one, two

    allocate (two%a)
    one = two
  end subroutine test2
end program test

[Bug fortran/57904] Bogus(?) "invokes undefined behavior" warning with Fortran's finalization wrapper (gfortran.dg/class_48.f90)

[jamborm at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57904

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-07-23
                 CC|                            |jakub at gcc dot gnu.org,
                   |                            |jamborm at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #1 from Martin Jambor <jamborm at gcc dot gnu.org> ---
I'm not really sure what the warning is about.  The warning is emitted
in the cunrolli (note the i at the end) pass when it also dumps the
following to the dump:

Statement _16 = idx_15 + -1;
 is executed at most 2147483647 (bounded by 2147483647) + 1 times in loop 3.

which looks suspicious.  However, the whole loop is guarded by
cndition (ubound.0_3 > 0) which IPA-CP tells us is false, so the code
is never executed.  Indeed the whole loop disappears in the very next
pass dump ccp2 and scheduling an extra ccp before cunrolli makes the
warning go away.

Having said that, I'm not sure how to proceed at the moment and have
to leave my office pretty much immediately :-)  Parhaps Jakub, who
introduced the warning, might have an idea?

[Bug fortran/57904] Bogus(?) "invokes undefined behavior" warning with Fortran's finalization wrapper (gfortran.dg/class_48.f90)

[ro at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57904

Rainer Orth <ro at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ro at gcc dot gnu.org
   Target Milestone|---                         |4.9.0

[Bug fortran/57904] [4.9 Regression] Bogus(?) "invokes undefined behavior" warning with Fortran's finalization wrapper (gfortran.dg/class_48.f90)

[hp at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57904

Hans-Peter Nilsson <hp at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hp at gcc dot gnu.org
            Summary|Bogus(?) "invokes undefined |[4.9 Regression] Bogus(?)
                   |behavior" warning with      |"invokes undefined
                   |Fortran's finalization      |behavior" warning with
                   |wrapper                     |Fortran's finalization
                   |(gfortran.dg/class_48.f90)  |wrapper
                   |                            |(gfortran.dg/class_48.f90)

--- Comment #2 from Hans-Peter Nilsson <hp at gcc dot gnu.org> ---
I'm guessing this fails for all ILP32 targets as described; at least ARM, CRIS,
PowerPC and m68k as well.  Adjusting title to properly mark as regression.

[Bug fortran/57904] [4.9 Regression] Bogus(?) "invokes undefined behavior" warning with Fortran's finalization wrapper (gfortran.dg/class_48.f90)

[bernd.edlinger at hotmail dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57904

Bernd Edlinger <bernd.edlinger at hotmail dot de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bernd.edlinger at hotmail dot de

--- Comment #3 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
this is somehow very similar to PR58143.
also here, the warning goes away at -O2 and -Os
if I add -fno-strict-overflow or -ftrapv or -fwrapv...

[Bug middle-end/57904] [4.9 Regression] Bogus(?) "invokes undefined behavior" warning with Fortran's finalization wrapper (gfortran.dg/class_48.f90)

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57904

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1
          Component|fortran                     |middle-end

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
But not fixed by the fix for PR58143 and not present on the branch.

Re-confirmed.

Loop 1 iterates at most 2147483646 times.
Loop 3 iterates 4294967295 times.
Loop 3 iterates at most -1 times.

still needs investigation.

[Bug middle-end/57904] [4.9 Regression] Bogus(?) "invokes undefined behavior" warning with Fortran's finalization wrapper (gfortran.dg/class_48.f90)

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57904

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
We have:
<bb 2>:
ubound.0_3 = 0;
size.1_4 = ubound.0_3 + 1;
size.1_5 = MAX_EXPR <size.1_4, 0>;
_6 = size.1_5 * 4;
_7 = (character(kind=4)) _6;
_8 = MAX_EXPR <_7, 1>;
sizes_9 = __builtin_malloc (_8);
size.3_10 = MAX_EXPR <ubound.0_3, 0>;
_11 = size.3_10 * 4;
_12 = (character(kind=4)) _11;
_13 = MAX_EXPR <_12, 1>;
strides_14 = __builtin_malloc (_13);
MEM[(integer(kind=4)[0:D.1906] *)sizes_9][0] = 1;
if (ubound.0_3 > 0)
  goto <bb 3>;
else
  goto <bb 6>;
<bb 3>:
idx_50 = 1;
<bb 4>:
# idx_15 = PHI <1(3), idx_25(5)>
_16 = idx_15 + -1;
_17 = array_1(D)->dim[_16].stride;
MEM[(integer(kind=4)[0:D.1900] *)strides_14][_16] = _17;
_18 = MEM[(integer(kind=4)[0:D.1906] *)sizes_9][_16];
_19 = array_1(D)->dim[_16].ubound;
_20 = array_1(D)->dim[_16].lbound;
_21 = _19 - _20;
_22 = _21 + 1;
_23 = MAX_EXPR <_22, 0>;
_24 = _18 * _23;
MEM[(integer(kind=4)[0:D.1906] *)sizes_9][idx_15] = _24;
idx_25 = idx_15 + 1;
if (ubound.0_3 == idx_15)
  goto <bb 6>;
else
  goto <bb 5>;
<bb 5>:
goto <bb 4>;

and the warning is on the header(4) latch(5) loop.  The warning is correct,
the loop would execute 0xffffffff times and _16 = idx_15 + -1 would invoke
undefined behavior on the last iteration, but still the warning is undesirable
because it is on a dead loop.  Only during IPA optimizations the original
  _15 = array_14(D)->dtype;
  ubound.0_16 = _15 & 7;
was optimized into:
  _2 = 344;
  ubound.0_3 = _2 & 7;
and only copyprop2 pass optimizes that into:
  ubound.0_3 = 0;
but nothing yet propagated it into the condition and folded the condition,
cunrolli runs simply too early.  I guess scheduling there a forwprop pass in
between copyprop2 and cunrolli would fix this, but don't know how expensive it
is.  Or we could avoid emitting the warning right away, but add there
__builtin_warning or similar into the loop, and only warn later on after some
cleanup passes that can actually figure out what code is dead.

[Bug middle-end/57904] [4.9 Regression] Bogus(?) "invokes undefined behavior" warning with Fortran's finalization wrapper (gfortran.dg/class_48.f90)

[Joost.VandeVondele at mat dot ethz.ch]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57904

Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |Joost.VandeVondele at mat dot ethz
                   |                            |.ch

--- Comment #6 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
this is the same/similar issue as PR58746, which also triggers this warning but
even without -m32 on x86_64.

[Bug middle-end/57904] [4.9 Regression] Bogus(?) "invokes undefined behavior" warning with Fortran's finalization wrapper (gfortran.dg/class_48.f90)

[law at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57904

--- Comment #8 from Jeffrey A. Law <law at redhat dot com> ---
I wonder if we could refactor propgate_rhs_into_lhs from tree-ssa-dom.c to help
here.  It was designed to handle precisely this kind of problem.

[Bug middle-end/57904] [4.9 Regression] Bogus(?) "invokes undefined behavior" warning with Fortran's finalization wrapper (gfortran.dg/class_48.f90)

[bernd.edlinger at hotmail dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57904

--- Comment #9 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
Created attachment 31485
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=31485&action=edit
change code generation for simple DO-loops

This not yet fully tested patch changes the DO-loop code generation
to a more loop-niter frendly way.

before:
   if ((step > 0) ? (dovar <= to) : (dovar => to))
    {
      for (;;)
        {
          body;
   cycle_label:
          cond = (dovar == to);
          dovar += step;
          if (cond) goto end_label;
        }
      }
   end_label:


after:
   if ((step > 0) ? (dovar <= to) : (dovar => to))
    {
      for (;;)
        {
          body;
   cycle_label:
          cond = (step > 0) ? (dovar >= to) : (dovar <= to);
          dovar += step;
          if (cond) goto end_label;
        }
      }
   end_label:

this is more easy to see, that dovar can not overflow:
what do you think of it?

[Bug middle-end/57904] [4.9 Regression] Bogus(?) "invokes undefined behavior" warning with Fortran's finalization wrapper (gfortran.dg/class_48.f90)

[law at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57904

--- Comment #10 from Jeffrey A. Law <law at redhat dot com> ---
Bernd,

It's certainly good if the test outside the loop and inside the loop is the
same.  It's a lot more likely to be discovered to be redundant earlier.  I have
no idea how it would affect ivopts.

Regardless of what the front-end presents us with, the early optimizers are
just doing a crappy job here.  If you look at the IL dump and
propagate/simplify based on ubound_3 = 0 you'll find that a ton of code just
goes away.

The question in my mind is can we get the propagation/simplifications we want
with a reasonable cost.  We certainly have passes that will clean this up that
we could schedule to run after copyprop, but an integrated solution may be
significantly better from a compile-time standpoint.

It also helps that we already have code which does, probably 95% of what we
need in phi-only cprop.  I suspect if we just marked things that obviously
simplified down to copies/constants and re-used the phi-only-cprop code that
everything would "just work" with a minimal compile-time hit.  I'm hoping to
prototype that tomorrow.

[Bug middle-end/57904] [4.9 Regression] Bogus(?) "invokes undefined behavior" warning with Fortran's finalization wrapper (gfortran.dg/class_48.f90)

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57904

--- Comment #11 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
With the patch in comment 9, gfortran.dg/class_48.f90 no longer fails and I
don't see any regression. The warning for the test in pr58746 comment 2 is also
fixed.

[Bug middle-end/57904] [4.9 Regression] Bogus(?) "invokes undefined behavior" warning with Fortran's finalization wrapper (gfortran.dg/class_48.f90)

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57904

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Dominique d'Humieres from comment #11)
> With the patch in comment 9, gfortran.dg/class_48.f90 no longer fails and I
> don't see any regression. The warning for the test in pr58746 comment 2 is
> also fixed.

But you can always create testcases (in C/C++ etc.) that will hit this warning,
so while the FE change is possible, we need to do something either about the
optimization passes in between IPA and cunrolli (copyprop change Jeff talks
about, perhaps only done for that single pass instance and not others, or all?,
guess depending on how expensive it is) or scheduling there another instance of
some other cleanup pass, or deferring the warning reporting until some cleanup.

For the FE change, I guess most important are benchmark results, doesn't it
slow down important benchmarks?

[Bug middle-end/57904] [4.9 Regression] Bogus(?) "invokes undefined behavior" warning with Fortran's finalization wrapper (gfortran.dg/class_48.f90)

[bernd.edlinger at hotmail dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57904

--- Comment #13 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
(In reply to Jakub Jelinek from comment #12)
> But you can always create testcases (in C/C++ etc.) that will hit this
> warning, so while the FE change is possible, we need to do something either
> about the optimization passes in between IPA and cunrolli (copyprop change
> Jeff talks about, perhaps only done for that single pass instance and not
> others, or all?, guess depending on how expensive it is) or scheduling there
> another instance of some other cleanup pass, or deferring the warning
> reporting until some cleanup.
> 

Could someone please provide a C test case that generates this warning?

maybe something like:

 int dovar = from;
 if (dovar <= to)
   for (;;)
     {
      ...
      if (dovar == to)
        break;
      dovar += step;
    }

> For the FE change, I guess most important are benchmark results, doesn't it
> slow down important benchmarks?


I think the change in the simple DO-loops really makes sense,
independent of this warning.

It generates the IL code just a bit more like it is done in C:

for (dovar=from; dovar <= to; dovar+=step) { ... }

=>

 dovar = from;
 if (dovar <= to)
 {
   loop:
    ...
    dovar+=step;
    if (dovar <= to)
       goto loop;
 }

Therefore the IL is easier to analyze and 
chances are good that the optimizer performs better.

I can't do these benchmarks however. Who could do that for us?

[Bug middle-end/57904] [4.9 Regression] Bogus(?) "invokes undefined behavior" warning with Fortran's finalization wrapper (gfortran.dg/class_48.f90)

[law at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57904

--- Comment #14 from Jeffrey A. Law <law at redhat dot com> ---
So a quick prototype which reuses the infrastructure from the
phi-only-propagator cleans things up quite nicely.

Given this block after substitute_and_fold does its thing:

<bb 2>:
ubound.0_3 = 0;
size.1_4 = ubound.0_3 + 1;
size.1_5 = MAX_EXPR <size.1_4, 0>;
_6 = size.1_5 * 4;
_7 = (character(kind=4)) _6;
_8 = MAX_EXPR <_7, 1>;
sizes_9 = __builtin_malloc (_8);
size.3_10 = MAX_EXPR <ubound.0_3, 0>;
_11 = size.3_10 * 4;
_12 = (character(kind=4)) _11;
_13 = MAX_EXPR <_12, 1>;
strides_14 = __builtin_malloc (_13);
MEM[(integer(kind=4)[0:D.1917] *)sizes_9][0] = 1;
if (ubound.0_3 > 0)
  goto <bb 3>;
else
  goto <bb 6>;


I (manually) seed the phi-only propagator's 2nd step with _3 as being a newly
exposed destination of a copy/constant initialization and let that trivial
propagator do its thing...  Resulting in:

<bb 2>:
sizes_9 = __builtin_malloc (4);
strides_14 = __builtin_malloc (1);
MEM[(integer(kind=4)[0:D.1917] *)sizes_9][0] = 1;
goto <bb 6>;

Which is exactly what we want.  Also note that we collapse the conditional at
the end of the block.  That in turn makes the problematic loop unreachable and
it goes away as one would expect.

The problem I see is I'd prefer not to expose this in substitute_and_fold
directly.  That routine is used by multiple propagators.  I'm thinking that
instead we can have a callback to the pass utilizing substitute_and_fold that
gets called when something has folded.

[Bug middle-end/57904] [4.9 Regression] Bogus(?) "invokes undefined behavior" warning with Fortran's finalization wrapper (gfortran.dg/class_48.f90)

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57904

--- Comment #15 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
*** Bug 58746 has been marked as a duplicate of this bug. ***

[Bug middle-end/57904] [4.9 Regression] Bogus(?) "invokes undefined behavior" warning with Fortran's finalization wrapper (gfortran.dg/class_48.f90)

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57904

--- Comment #16 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
> For the FE change, I guess most important are benchmark results, 
> doesn't it slow down important benchmarks?

AFAICT the answer is not at least for the gfortran test suite and the
polyhedron ones
(pb05, pb11, and my own variants).

May be Joost can do more testing for CP2K.

[Bug middle-end/57904] [4.9 Regression] Bogus(?) "invokes undefined behavior" warning with Fortran's finalization wrapper (gfortran.dg/class_48.f90)

[law at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57904

--- Comment #17 from Jeffrey A. Law <law at redhat dot com> ---
Dominique, thanks for verifying that 58746 is a duplicate.  I was wondering
about that.

Richi, we've known for a long time (since the early 90s) that running CSE soon
after loop unrolling is profitable.  Soooo, why not run conditionally run DOM
after unrolling?   That wouldn't create a compile-time hit for a typical -O2
compilation.

[Bug middle-end/57904] [4.9 Regression] Bogus(?) "invokes undefined behavior" warning with Fortran's finalization wrapper (gfortran.dg/class_48.f90)

[law at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57904

--- Comment #18 from Jeffrey A. Law <law at redhat dot com> ---
Whoops, message for Richi was meant for a different BZ.

[Bug middle-end/57904] [4.9 Regression] Bogus(?) "invokes undefined behavior" warning with Fortran's finalization wrapper (gfortran.dg/class_48.f90)

[law at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57904

--- Comment #19 from Jeffrey A. Law <law at gcc dot gnu.org> ---
Author: law
Date: Fri Jan 17 17:50:10 2014
New Revision: 206723

URL: http://gcc.gnu.org/viewcvs?rev=206723&root=gcc&view=rev
Log:
    PR middle-end/57904
    * passes.def: Reorder pass_copy_prop, pass_unrolli, pass_ccp sequence
    so that pass_ccp runs first.

        PR middle-end/57904
    * gfortran.dg/pr57904.f90: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/pr57904.f90
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/passes.def
    trunk/gcc/testsuite/ChangeLog

[Bug middle-end/57904] [4.9 Regression] Bogus(?) "invokes undefined behavior" warning with Fortran's finalization wrapper (gfortran.dg/class_48.f90)

[law at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57904

Jeffrey A. Law <law at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #20 from Jeffrey A. Law <law at redhat dot com> ---
Fix by recent commit.