This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
PATCH: Fix PR2162
- To: gcc-patches at gcc dot gnu dot org
- Subject: PATCH: Fix PR2162
- From: Mark Mitchell <mark at codesourcery dot com>
- Date: Wed, 02 May 2001 19:13:48 -0700
- Cc: Phil Edwards <pedwards at jaj dot com>, Richard Henderson <rth at cygnus dot com>
- Organization: CodeSourcery, LLC
Here's another draft patch for the SPARC. Phil, if you're willing to
run another bootstrap/test cycle when the one you're doing gets done,
that would be excellent. Richard, borrowing a few more of your brain
cycles would be cool.
The problem is that loop-8.c failed with -O3 -fPIC on the SPARC. The
bottom line is that loop decided to reverse this loop:
for (d = 0; d < 3; d++)
{
c = a[d];
if (c > 0.0) goto e;
}
Uncool.
The problem is this conditional:
if ((num_nonfixed_reads <= 1
&& ! loop_info->has_nonconst_call
&& ! loop_info->has_volatile
&& reversible_mem_store
&& (bl->giv_count + bl->biv_count + loop_info->num_mem_sets
+ LOOP_MOVABLES (loop)->num + compare_and_branch == insn_count)
&& (bl == ivs->list && bl->next == 0))
|| no_use_except_counting)
in check_dbra_loop. The important part is the bit that tries to
determine that all the instructions in the loop are accounted for:
they are either set bivs or givs based on the loop counter, set
memory, are a movable instruction, etc.
The problem is that LOOP_MOVABLES (loop)->num at this point refers to
all the movables in existence -- including some that have been moved
out of the loop -- but insn_count only corresponds to instructions
within the loop. So, we think all the instructions are accounted for,
but that's not really correct.
The logic setting ->num seemed fragile, so I decided just to compute
the right number when we need it. I verified that this fixed the
loop-8 bug by hand-inspecting the assembly code, but I'm not sure that
I'm a compliant SPARC processor. I also checked that we still reverse
some simple loops.
If I got things wrong, we could end up reversing other loops that
we're not supposed to, which would be bad. (The other failure mode --
not reversing things we should -- is not as bad.) So, even though I'm
bootstrapping on i86-pc-linux-gnu, I'd appreciate a SPARC bootstrap as
well.
Thanks,
--
Mark Mitchell mark@codesourcery.com
CodeSourcery, LLC http://www.codesourcery.com
Index: loop.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/loop.c,v
retrieving revision 1.322.4.7
diff -c -p -r1.322.4.7 loop.c
*** loop.c 2001/04/25 15:29:38 1.322.4.7
--- loop.c 2001/05/03 02:06:13
*************** static void ignore_some_movables PARAMS
*** 166,171 ****
--- 166,172 ----
static void force_movables PARAMS ((struct loop_movables *));
static void combine_movables PARAMS ((struct loop_movables *,
struct loop_regs *));
+ static int num_unmoved_movables PARAMS ((const struct loop *));
static int regs_match_p PARAMS ((rtx, rtx, struct loop_movables *));
static int rtx_equal_for_loop_p PARAMS ((rtx, rtx, struct loop_movables *,
struct loop_regs *));
*************** scan_loop (loop, flags)
*** 551,557 ****
movables->head = 0;
movables->last = 0;
- movables->num = 0;
/* Determine whether this loop starts with a jump down to a test at
the end. This will occur for a small number of loops with a test
--- 552,557 ----
*************** combine_movables (movables, regs)
*** 1422,1427 ****
--- 1422,1445 ----
/* Clean up. */
free (matched_regs);
}
+
+ /* Returns the number of movable instructions in LOOP that were not
+ moved outside the loop. */
+
+ static int
+ num_unmoved_movables (loop)
+ const struct loop *loop;
+ {
+ int num = 0;
+ struct movable *m;
+
+ for (m = LOOP_MOVABLES (loop)->head; m; m = m->next)
+ if (!m->done)
+ ++num;
+
+ return num;
+ }
+
/* Return 1 if regs X and Y will become the same if moved. */
*************** move_movables (loop, movables, threshold
*** 1635,1642 ****
rtx *reg_map = (rtx *) xcalloc (nregs, sizeof (rtx));
char *already_moved = (char *) xcalloc (nregs, sizeof (char));
- movables->num = 0;
-
for (m = movables->head; m; m = m->next)
{
/* Describe this movable insn. */
--- 1653,1658 ----
*************** move_movables (loop, movables, threshold
*** 1665,1673 ****
INSN_UID (m->forces->insn));
}
- /* Count movables. Value used in heuristics in strength_reduce. */
- movables->num++;
-
/* Ignore the insn if it's already done (it matched something else).
Otherwise, see if it is now safe to move. */
--- 1681,1686 ----
*************** check_dbra_loop (loop, insn_count)
*** 7363,7369 ****
&& ! loop_info->has_volatile
&& reversible_mem_store
&& (bl->giv_count + bl->biv_count + loop_info->num_mem_sets
! + LOOP_MOVABLES (loop)->num + compare_and_branch == insn_count)
&& (bl == ivs->list && bl->next == 0))
|| no_use_except_counting)
{
--- 7376,7382 ----
&& ! loop_info->has_volatile
&& reversible_mem_store
&& (bl->giv_count + bl->biv_count + loop_info->num_mem_sets
! + num_unmoved_movables (loop) + compare_and_branch == insn_count)
&& (bl == ivs->list && bl->next == 0))
|| no_use_except_counting)
{
Index: loop.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/loop.h,v
retrieving revision 1.51
diff -c -p -r1.51 loop.h
*** loop.h 2001/01/25 09:28:55 1.51
--- loop.h 2001/05/03 02:06:14
*************** struct loop_movables
*** 289,296 ****
struct movable *head;
/* Last movable in chain. */
struct movable *last;
- /* Number of movables in the loop. */
- int num;
};
--- 289,294 ----