[Bug fortran/45505] [4.6 Regression] gfortran.dg/pr25923.f90
--- 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
--- 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
--- 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
[Bug fortran/45505] [4.6 Regression] gfortran.dg/pr25923.f90
--- 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
--- 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
--- 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
--- 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
--- 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
--- 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=gccview=revrev=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
--- 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|-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
-- 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
--- 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
