[Bug fortran/45505] New: [4.6 Regression] gfortran.dg/pr25923.f90

[Bug fortran/45505] New: [4.6 Regression] gfortran.dg/pr25923.f90

[hjl dot tools at gmail dot com]

On Linux/x86, revision 163781 gave

FAIL: gfortran.dg/pr25923.f90  -O   (test for warnings, line 13)
FAIL: gfortran.dg/pr25923.f90  -O  (test for excess errors)

Revision 163775 is OK. It may be caused by revision 163776:

http://gcc.gnu.org/ml/gcc-cvs/2010-09/msg00067.html


-- 
           Summary: [4.6 Regression] gfortran.dg/pr25923.f90
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: hjl dot tools at gmail dot com


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

[Bug fortran/45505] [4.6 Regression] gfortran.dg/pr25923.f90

[burnus at gcc dot gnu dot org]

------- Comment #1 from burnus at gcc dot gnu dot org  2010-09-02 18:32 -------
That's a -m32 (fails) vs. -m64 (works) issue, cf.
http://gcc.gnu.org/ml/fortran/2010-09/msg00062.html


-- 


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

[Bug fortran/45505] [4.6 Regression] gfortran.dg/pr25923.f90

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.6.0


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

[Bug fortran/45505] [4.6 Regression] gfortran.dg/pr25923.f90

[hp at gcc dot gnu dot org]

------- Comment #2 from hp at gcc dot gnu dot org  2010-09-06 22:02 -------
I think I'll try doing it the IIUC documented preferred way for deferred bugs;
to xfail the test-case for ilp32 targets with a reference to this PR.  And yes,
seen by the cris-elf autotester too.


-- 

hp at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hp at gcc dot gnu dot org
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2010-09-06 22:02:19
               date|                            |


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

[Bug fortran/45505] [4.6 Regression] gfortran.dg/pr25923.f90

[hp at gcc dot gnu dot org]

------- Comment #3 from hp at gcc dot gnu dot org  2010-09-07 07:11 -------
(In reply to comment #2)
> xfail the test-case for ilp32 targets with a reference to this PR.

<http://gcc.gnu.org/ml/fortran/2010-09/msg00179.html>


-- 


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

[Bug fortran/45505] [4.6 Regression] gfortran.dg/pr25923.f90

[jakub at gcc dot gnu dot org]

------- Comment #4 from jakub at gcc dot gnu dot org  2010-09-07 08:05 -------
This isn't specific to Fortran,
struct S { int a; };

struct S foo (struct S *arg)
{
  struct S s;
  if (bar ())
    baz ("not valid");
  else
    s = *arg;
  return s;
}

has the exactly same difference.
I believe this is caused by ESRA or gimplification.  In *.ealias we have:
<bb 5>:
  [pr25923.f90 : 22:0] D.1570 = res;
  return D.1570;
and in *.esra this is:
<bb 5>:
  # res$yr_9 = PHI <res$yr_8(D)(3), [pr25923.f90 : 20:0] res$yr_3(4)>
  [pr25923.f90 : 22:0] SR.2_10 = res$yr_9;
  D.1570.yr = SR.2_10;
  return D.1570;

Note no locus on D.1570.yr = SR.2_10; assignment.  The question is if ESRA just
forgots to set the locus, or if it sets it from the following stmt
(GIMPLE_RETURN).  If the former, then it would just be a SRA bug, if the
latter,
then the question is why don't we set location on GIMPLE_RETURN.
Note that on the C testcase there apparently is locus on GIMPLE_RETURN between
gimplification and gimple lowering (where the locus on it is lost).
On the Fortran testcase in *.gimple the locus is strange:
  [pr25923.f90 : 22:0] D.1570 = res;
  [pr25923.f90 : 13:0] return D.1570;
Line 22 is the END FUNCTION line, line 13 is the FUNCTION line.  For C it is
obvious where it wants to report the return, for Fortran, given that it doesn't
have any kind of RETURN statement, I guess either of the locations is fine, but
we should be consistent.


-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jamborm at gcc dot gnu dot
                   |                            |org


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

[Bug fortran/45505] [4.6 Regression] gfortran.dg/pr25923.f90

[hp at gcc dot gnu dot org]

------- Comment #5 from hp at gcc dot gnu dot org  2010-09-07 13:28 -------
Subject: Bug 45505

Author: hp
Date: Tue Sep  7 13:23:24 2010
New Revision: 163949

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=163949
Log:
        PR fortran/45505
        * gfortran.dg/pr25923.f90: XFAIL warning on wrong line for ilp32.

Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/pr25923.f90


-- 


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

[Bug fortran/45505] [4.6 Regression] gfortran.dg/pr25923.f90

[jamborm at gcc dot gnu dot org]

------- Comment #6 from jamborm at gcc dot gnu dot org  2010-09-14 13:55 -------
Jakub, I have not understood whether you think the warning emitted
when compiling the c code from comment #4 has the correct line number
or not.  I see it attributed to the line with the return statement
which I think is correct.

Nevertheless, SRA currently does not set locations of gimple
statements it generates and is quite confused when it comes to tree
locations.  I was already looking into the latter problem and added
setting gimple statement locations appropriately because it fitted in
well.  The patch
(http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01175.html) has just
been approved and I am going to commit it in a few moments but it does
not change the line the Fortran warning is reported at.  It is
probably because the gimple return statement, which is where I get the
location for the newly generated load, has a location corresponding to
the end of the function.


-- 


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

[Bug fortran/45505] [4.6 Regression] gfortran.dg/pr25923.f90

[jakub at gcc dot gnu dot org]

------- Comment #7 from jakub at gcc dot gnu dot org  2010-09-17 12:46 -------
You now set the location, but I believe the wrong one.
esra changes are:
 foo (struct S * arg)
 {
+  int SR.1;
+  int s$a;
   struct S s;
   struct S D.2694;
   int D.2690;
@@ -72,10 +37,12 @@ foo (struct S * arg)
   goto <bb 5>;

 <bb 4>:
-  [pr25923.c : 13:7] s = [pr25923.c : 13] *arg_2(D);
+  [pr25923.c : 13:7] s$a_9 = [pr25923.c : 13] MEM[(struct S *)arg_2(D)];

 <bb 5>:
-  [pr25923.c : 14:3] D.2694 = s;
+  # s$a_8 = PHI <s$a_7(D)(3), [pr25923.c : 13:7] s$a_9(4)>
+  [pr25923.c : 14:3] SR.1_10 = s$a_8;
+  MEM[(struct S *)&D.2694] = SR.1_10;
   return D.2694;

 }

The added MEM = SR.1_10 uses location_t of the stmt after it, as
sra_modify_expr
emits statements before gsi.  I'm arguing that in this case the MEM[(struct S
*)&D.2694] = SR.1_10; stmt is not related to return D.2694, but to D.2694 = s
that has been removed and thus I believe it should inherit its locus as well.


-- 


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

[Bug fortran/45505] [4.6 Regression] gfortran.dg/pr25923.f90

[jamborm at gcc dot gnu dot org]

------- Comment #8 from jamborm at gcc dot gnu dot org  2010-09-17 15:08 -------
(In reply to comment #7)
> 
> The added MEM = SR.1_10 uses location_t of the stmt after it, as
> sra_modify_expr
> emits statements before gsi.  I'm arguing that in this case the MEM[(struct S
> *)&D.2694] = SR.1_10; stmt is not related to return D.2694, but to D.2694 = s
> that has been removed and thus I believe it should inherit its locus as well.
> 

Unfortunately no, the statement SR.1_10 = s$a_8; is produced when
replacing the original assignment, MEM[(struct S *)&D.2694] = SR.1_10;
is created when processing the return statement, it is updating the
original agregate D.2694 with data in its replacements (in this case
there is only one but there can be more) before it is used as a whole.
I can't see how it could be otherwise.

It would be difficult also to set the location of the MEM_REF
statement according to the definition of its RHS because when the
statement is built the RHS is not yet in SSA form.

Thinking about it more, the RHS in that statement, SR.1_10 has its
TREE_NO_WARNING set and so that line should not be a problem at this
stage.  So I guess s$a_8 is propagated there at some point later.
Perhaps the fwprop could change the location when it substitutes an
operand in situations like this?  I know it would be a bit ugly but
doing the same in SRA would require some aggregate data flow analysis
which would be quite hard.


-- 


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

[Bug fortran/45505] [4.6 Regression] gfortran.dg/pr25923.f90

[jamborm at gcc dot gnu dot org]

------- Comment #9 from jamborm at gcc dot gnu dot org  2010-09-22 11:00 -------
I have just examined the Fortran testcase more thoroughly and to my
surprise I realized SRA did not create any new statements on i686.  It
merely changed statements

res = *arg_1(D); into res$yr = MEM[(struct bar &)arg_1(D)]; and
<retval> = res; into MEM[(struct bar *)&<retval>] = res$yr;

both by simply changing the LHS and RHS.  Needless to say, the
locations of the statements remain unchanged.  Locations for the new
memory references have been taken from the statements.


-- 


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

[Bug fortran/45505] [4.6 Regression] gfortran.dg/pr25923.f90

[jamborm at gcc dot gnu dot org]

------- Comment #10 from jamborm at gcc dot gnu dot org  2010-09-22 15:33 -------
gimple_has_location returns false for the return statement on both
i686 and x86_64.  When I hacked SRA to set the location of return
statement to the location of the preceding statement 
("<retval> = res;" or "D.1523 = res;"), the reported lines in the
warnings were consistent on both i686 and x86_64 (22 in both cases, of
course).

So making sure the two statements both have the location we want to
give in the warning is a way of dealing with this issue.  To me it
even seems they do belong to the same source construct and should have
the same location.


-- 


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

[Bug fortran/45505] [4.6 Regression] gfortran.dg/pr25923.f90

[jakub at gcc dot gnu dot org]

------- Comment #11 from jakub at gcc dot gnu dot org  2010-09-22 15:59 -------
The reason why the return stmt, at least after lowering, doesn't have a
location, is because after lowering there is just one return instead of
possibly multiple returns from before lowering.  So the location_t of the
individual returns is preserved on the gimple assignments to the RESULT_DECL.

What I just find strange is why is the return stmt involved in the SRA
optimization (except as unrelated stmt following the deleted stmt).  There was
an assignment to RESULT_DECL before that, it had the intended locus of the
return from the source, and I'd say that the replacements are connected to that
statements if the RESULT_DECL can't be scalarized.


-- 


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