| Message ID | 20160526110205.GJ3300@bubble.grove.modra.org |
|---|---|
| State | New |
| Headers | show |
On 05/26/2016 07:02 AM, Alan Modra wrote: > This fixes lack of bb_loop_depth info in some of the early parts of > ira, which has been the case for quite some time. All active branches > return 0 from bb_loop_depth() in update_equiv_regs, but whether that > actually causes mis-optimization anywhere but trunk is yet to be > determined. > > I played a little with trying to consolidate this loop_optimizer_init > call with one that occurs a little later, but ran into ICEs. (We now > have four calls to loop_optimizer_init in ira.c.) > > Bootstrapped and regression tested powerpc64le-linux and x86_64-linux. > OK to apply? > Yes. Thank you, Alan.
On Thu, May 26, 2016 at 10:12:14AM -0400, Vladimir Makarov wrote: > On 05/26/2016 07:02 AM, Alan Modra wrote: > >This fixes lack of bb_loop_depth info in some of the early parts of > >ira, which has been the case for quite some time. All active branches > >return 0 from bb_loop_depth() in update_equiv_regs, but whether that > >actually causes mis-optimization anywhere but trunk is yet to be > >determined. > > > >I played a little with trying to consolidate this loop_optimizer_init > >call with one that occurs a little later, but ran into ICEs. (We now > >have four calls to loop_optimizer_init in ira.c.) > > > >Bootstrapped and regression tested powerpc64le-linux and x86_64-linux. > >OK to apply? > > > Yes. Thank you, Alan. Hi Vlad, Sorry to do this to you and others, but the patch (committed as r236789) may be wrong. I didn't see any problems on trunk but when I backported to gcc-5, I hit an error in stage2 compiling insn-recog.c "dominator of 10 status unknown" from if_after_reload. On gcc-5, the error disappears by adding a call to free_dominance_info (CDI_DOMINATORS); after the newly added call to loop_optimizer_finalize. I'm not sure yet what is going on. Does anyone know whether the free_dominance_info call is needed on trunk?
On 05/26/2016 10:14 PM, Alan Modra wrote: > On Thu, May 26, 2016 at 10:12:14AM -0400, Vladimir Makarov wrote: >> On 05/26/2016 07:02 AM, Alan Modra wrote: >>> This fixes lack of bb_loop_depth info in some of the early parts of >>> ira, which has been the case for quite some time. All active branches >>> return 0 from bb_loop_depth() in update_equiv_regs, but whether that >>> actually causes mis-optimization anywhere but trunk is yet to be >>> determined. >>> >>> I played a little with trying to consolidate this loop_optimizer_init >>> call with one that occurs a little later, but ran into ICEs. (We now >>> have four calls to loop_optimizer_init in ira.c.) >>> >>> Bootstrapped and regression tested powerpc64le-linux and x86_64-linux. >>> OK to apply? >>> >> Yes. Thank you, Alan. > Hi Vlad, > Sorry to do this to you and others, but the patch (committed as > r236789) may be wrong. I didn't see any problems on trunk but when > I backported to gcc-5, I hit an error in stage2 compiling > insn-recog.c "dominator of 10 status unknown" from if_after_reload. > > On gcc-5, the error disappears by adding a call to > free_dominance_info (CDI_DOMINATORS); > after the newly added call to loop_optimizer_finalize. > > I'm not sure yet what is going on. Does anyone know whether the > free_dominance_info call is needed on trunk? > That is ok. It is always a discovery. I am not sure but I think I saw this problem when I wrote IRA. Looking at the dominance code, I seems to me that it can reuse the previous info if it was not cleared. So I guess free_dominance_info is important.
On Thu, May 26, 2016 at 4:02 AM, Alan Modra <amodra@gmail.com> wrote: > This fixes lack of bb_loop_depth info in some of the early parts of > ira, which has been the case for quite some time. All active branches > return 0 from bb_loop_depth() in update_equiv_regs, but whether that > actually causes mis-optimization anywhere but trunk is yet to be > determined. > > I played a little with trying to consolidate this loop_optimizer_init > call with one that occurs a little later, but ran into ICEs. (We now > have four calls to loop_optimizer_init in ira.c.) > > Bootstrapped and regression tested powerpc64le-linux and x86_64-linux. > OK to apply? > > PR rtl-optimization/71275 > * ira.c (ira): Call loop_optimizer_init to set up bb_loop_depth > for update_equiv_regs and combine_and_move_insns. > This caused: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71531
On Tue, Jun 14, 2016 at 09:26:19AM -0700, H.J. Lu wrote: > On Thu, May 26, 2016 at 4:02 AM, Alan Modra <amodra@gmail.com> wrote: > > This fixes lack of bb_loop_depth info in some of the early parts of > > ira, which has been the case for quite some time. All active branches > > return 0 from bb_loop_depth() in update_equiv_regs, but whether that > > actually causes mis-optimization anywhere but trunk is yet to be > > determined. > > > > I played a little with trying to consolidate this loop_optimizer_init > > call with one that occurs a little later, but ran into ICEs. (We now > > have four calls to loop_optimizer_init in ira.c.) > > > > Bootstrapped and regression tested powerpc64le-linux and x86_64-linux. > > OK to apply? > > > > PR rtl-optimization/71275 > > * ira.c (ira): Call loop_optimizer_init to set up bb_loop_depth > > for update_equiv_regs and combine_and_move_insns. > > > > This caused: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71531 I have an x86 ABI question. Is it correct according to the ABI that parameter passing stack slots always have the same value on function exit as they had on entry? Between these two calls to _gfortran_string_verify, if (verify(c4, "A", back = .true.) .ne. 3) call abort if (verify(c4, "AB") .ne. 0) call abort it seems that gfortran is assuming that parameters passed on the stack are unchanged. _gfortran_string_verify is a C function, declared as follows in libgfortran/intrinsics/string_intrinsics_inc.c: gfc_charlen_type string_verify (gfc_charlen_type slen, const CHARTYPE *str, gfc_charlen_type setlen, const CHARTYPE *set, GFC_LOGICAL_4 back) This function happens to modify the stack slot for "slen" to -1 when "back" is true.
On Wed, Jun 15, 2016 at 11:00:22AM +0930, Alan Modra wrote: > On Tue, Jun 14, 2016 at 09:26:19AM -0700, H.J. Lu wrote: > > On Thu, May 26, 2016 at 4:02 AM, Alan Modra <amodra@gmail.com> wrote: > > > This fixes lack of bb_loop_depth info in some of the early parts of > > > ira, which has been the case for quite some time. All active branches > > > return 0 from bb_loop_depth() in update_equiv_regs, but whether that > > > actually causes mis-optimization anywhere but trunk is yet to be > > > determined. > > > > > > I played a little with trying to consolidate this loop_optimizer_init > > > call with one that occurs a little later, but ran into ICEs. (We now > > > have four calls to loop_optimizer_init in ira.c.) > > > > > > Bootstrapped and regression tested powerpc64le-linux and x86_64-linux. > > > OK to apply? > > > > > > PR rtl-optimization/71275 > > > * ira.c (ira): Call loop_optimizer_init to set up bb_loop_depth > > > for update_equiv_regs and combine_and_move_insns. > > > > > > > This caused: > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71531 So, not guilty. Or at least only guilty of exposing another bug. You have a problem with parameter passing. cat > pass.c <<EOF extern int __attribute__ ((pure)) f1 (int, int); int main (void) { if (f1 (1, 0) != 2) __builtin_abort (); if (f1 (1, 1) != 0) __builtin_abort (); return 0; } EOF cat > pass2.c <<EOF int __attribute__ ((pure)) f2 (int); int __attribute__ ((pure)) f1 (int a, int b) { int x = a; if (b) x = -a; return f2 (x + 1); } EOF cat > pass3.c <<EOF int __attribute__ ((pure)) f2 (int a) { return a; } EOF gcc -m32 -march=corei7 -mtune=slm -O2 pass.c pass2.c pass3.c ./a.out Aborted gcc -O2 pass.c pass2.c pass3.c ./a.out > I have an x86 ABI question. Is it correct according to the ABI that > parameter passing stack slots always have the same value on function > exit as they had on entry? > > Between these two calls to _gfortran_string_verify, > if (verify(c4, "A", back = .true.) .ne. 3) call abort > if (verify(c4, "AB") .ne. 0) call abort > it seems that gfortran is assuming that parameters passed on the stack > are unchanged. > > _gfortran_string_verify is a C function, declared as follows in > libgfortran/intrinsics/string_intrinsics_inc.c: > gfc_charlen_type > string_verify (gfc_charlen_type slen, const CHARTYPE *str, > gfc_charlen_type setlen, const CHARTYPE *set, > GFC_LOGICAL_4 back) > > This function happens to modify the stack slot for "slen" to -1 when > "back" is true. > > -- > Alan Modra > Australia Development Lab, IBM
On 06/15/2016 03:30 AM, Alan Modra wrote: > Between these two calls to _gfortran_string_verify, > if (verify(c4, "A", back = .true.) .ne. 3) call abort > if (verify(c4, "AB") .ne. 0) call abort > it seems that gfortran is assuming that parameters passed on the stack > are unchanged. How? Is this something in the Fortran frontend, or is there CSE going on for stores to the argument area? Bernd
On Wed, Jun 15, 2016 at 2:49 AM, Bernd Schmidt <bschmidt@redhat.com> wrote: > On 06/15/2016 03:30 AM, Alan Modra wrote: >> >> Between these two calls to _gfortran_string_verify, >> if (verify(c4, "A", back = .true.) .ne. 3) call abort >> if (verify(c4, "AB") .ne. 0) call abort >> it seems that gfortran is assuming that parameters passed on the stack >> are unchanged. > > > How? Is this something in the Fortran frontend, or is there CSE going on for > stores to the argument area? Is this related to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71532
On Wed, Jun 15, 2016 at 11:49:50AM +0200, Bernd Schmidt wrote: > On 06/15/2016 03:30 AM, Alan Modra wrote: > >Between these two calls to _gfortran_string_verify, > > if (verify(c4, "A", back = .true.) .ne. 3) call abort > > if (verify(c4, "AB") .ne. 0) call abort > >it seems that gfortran is assuming that parameters passed on the stack > >are unchanged. > > How? Is this something in the Fortran frontend, or is there CSE going on for > stores to the argument area? It's not a fortran problem, and I answered my own question about the ABI by the testcase in https://gcc.gnu.org/ml/gcc-patches/2016-06/msg01098.html. If tail calls are allowed, then the stack parameter area must be overwritten. Thus an ABI can't allow tail calls if it specifies the stack parameter area is preserved. x86 allows tail calls. I didn't look further into where the problem in arg setup occurs, sorry, except to note that gcc-5 does not have the problem.
On 06/15/2016 04:03 PM, Alan Modra wrote: > On Wed, Jun 15, 2016 at 11:49:50AM +0200, Bernd Schmidt wrote: >> On 06/15/2016 03:30 AM, Alan Modra wrote: >>> Between these two calls to _gfortran_string_verify, >>> if (verify(c4, "A", back = .true.) .ne. 3) call abort >>> if (verify(c4, "AB") .ne. 0) call abort >>> it seems that gfortran is assuming that parameters passed on the stack >>> are unchanged. >> >> How? Is this something in the Fortran frontend, or is there CSE going on for >> stores to the argument area? > > It's not a fortran problem, and I answered my own question about the > ABI by the testcase in > https://gcc.gnu.org/ml/gcc-patches/2016-06/msg01098.html. If tail > calls are allowed, then the stack parameter area must be overwritten. > Thus an ABI can't allow tail calls if it specifies the stack parameter > area is preserved. x86 allows tail calls. Well yes. The problem isn't that the stack area is overwritten, the problem is that something expects that it isn't, and it's not clear to me yet where that problem occurs. Bernd
On Wed, Jun 15, 2016 at 7:06 AM, Bernd Schmidt <bschmidt@redhat.com> wrote: > > > On 06/15/2016 04:03 PM, Alan Modra wrote: >> >> On Wed, Jun 15, 2016 at 11:49:50AM +0200, Bernd Schmidt wrote: >>> >>> On 06/15/2016 03:30 AM, Alan Modra wrote: >>>> >>>> Between these two calls to _gfortran_string_verify, >>>> if (verify(c4, "A", back = .true.) .ne. 3) call abort >>>> if (verify(c4, "AB") .ne. 0) call abort >>>> it seems that gfortran is assuming that parameters passed on the stack >>>> are unchanged. >>> >>> >>> How? Is this something in the Fortran frontend, or is there CSE going on >>> for >>> stores to the argument area? >> >> >> It's not a fortran problem, and I answered my own question about the >> ABI by the testcase in >> https://gcc.gnu.org/ml/gcc-patches/2016-06/msg01098.html. If tail >> calls are allowed, then the stack parameter area must be overwritten. >> Thus an ABI can't allow tail calls if it specifies the stack parameter >> area is preserved. x86 allows tail calls. > > > Well yes. The problem isn't that the stack area is overwritten, the problem > is that something expects that it isn't, and it's not clear to me yet where > that problem occurs. Will this patch https://gcc.gnu.org/bugzilla/attachment.cgi?id=38705 also fix this
diff --git a/gcc/ira.c b/gcc/ira.c index 55b4bd7..1b269ea 100644 --- a/gcc/ira.c +++ b/gcc/ira.c @@ -5171,6 +5171,7 @@ ira (FILE *f) ira_set_pseudo_classes (true, ira_dump_file); init_alias_analysis (); + loop_optimizer_init (AVOID_CFG_MODIFICATIONS); reg_equiv = XCNEWVEC (struct equivalence, max_reg_num ()); update_equiv_regs (); @@ -5186,6 +5187,7 @@ ira (FILE *f) if (optimize) add_store_equivs (); + loop_optimizer_finalize (); end_alias_analysis (); free (reg_equiv);