This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, stage1] Move insns without introducing new temporaries in loop2_invariant
- From: Steven Bosscher <stevenb dot gcc at gmail dot com>
- To: "Thomas Preud'homme" <thomas dot preudhomme at arm dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Eric Botcazou <ebotcazou at libertysurf dot fr>
- Date: Mon, 9 Mar 2015 12:48:26 +0100
- Subject: Re: [PATCH, stage1] Move insns without introducing new temporaries in loop2_invariant
- Authentication-results: sourceware.org; auth=none
- References: <000101d0572a$2de1f390$89a5dab0$ at arm dot com>
On Thu, Mar 5, 2015 at 10:53 AM, Thomas Preud'homme wrote:
> diff --git a/gcc/dominance.c b/gcc/dominance.c
> index 33d4ae4..09c8c90 100644
> --- a/gcc/dominance.c
> +++ b/gcc/dominance.c
> @@ -982,7 +982,7 @@ nearest_common_dominator_for_set (enum cdi_direction dir, bitmap blocks)
>
> A_Dominated_by_B (node A, node B)
> {
> - return DFS_Number_In(A) >= DFS_Number_In(A)
> + return DFS_Number_In(A) >= DFS_Number_In(B)
> && DFS_Number_Out (A) <= DFS_Number_Out(B);
> } */
This hunk is obvious enough ;-)
> diff --git a/gcc/loop-invariant.c b/gcc/loop-invariant.c
> index f79b497..ab2a45c 100644
> --- a/gcc/loop-invariant.c
> +++ b/gcc/loop-invariant.c
...
> + /* Check whether the set is always executed. We could omit this condition if
> + we know that the register is unused outside of the loop, but it does not
> + seem worth finding out. */
> + may_exit = BITMAP_ALLOC (NULL);
> + has_exit = BITMAP_ALLOC (NULL);
> + always_executed = BITMAP_ALLOC (NULL);
> + body = get_loop_body_in_dom_order (loop);
> + find_exits (loop, body, may_exit, has_exit);
> + compute_always_reached (loop, body, has_exit, always_executed);
> + /* Find bit position for basic block bb. */
> + for (i = 0; i < loop->num_nodes && body[i] != bb; i++);
> + if (!bitmap_bit_p (always_executed, i))
> + goto cleanup;
It looks like this would run for all candidate loop invariants, right?
If so, you're creating run time of O(n_invariants*n_bbs_in_loop), a
potential compile time hog for large loops.
But why compute this at all? Perhaps I'm missing something, but you
already have inv->always_executed available, no?
> + /* Check that all uses reached by the def in insn would still be reached
> + it. */
> + dest_regno = REGNO (reg);
> + for (use = DF_REG_USE_CHAIN (dest_regno); use; use = DF_REF_NEXT_REG (use))
> + {
> + rtx ref;
Would be nice if we can start using rtx_insn for new code. You do so
further up, I suggest you continue that good citizenship here :-)
> + basic_block use_bb;
> +
> + ref = DF_REF_INSN (use);
> + use_bb = BLOCK_FOR_INSN (ref);
You can use DF_REF_BB.
> + /* Ignore instruction considered for moving. */
> + if (ref == insn)
> + continue;
> +
> + /* Don't consider uses outside loop. */
> + if (!flow_bb_inside_loop_p (loop, use_bb))
> + continue;
> +
> + /* Don't move if a use is not dominated by def in insn. */
> + if (use_bb == bb && DF_INSN_LUID (insn) > DF_INSN_LUID (ref))
> + goto cleanup;
You're safer with ">=" even if you've already checked ref==insn.
Ciao!
Steven