On Tue, May 3, 2016 at 4:07 AM, Patrick Palka <patr...@parcs.ath.cx> wrote:
> This patch
>
> 1. changes the need_assert_for bitmap into an sbitmap since its size is
> known to be at most num_ssa_names
But then it is redundant anyway as we have asserts_for[num_ssa_names],
initialized to NULL. So bitmap_bit_p (need_assert_for, i) is
asserts_for[i] != NULL.
> 2. changes a use of FOR_EACH_IMM_USE_STMT to a use of
> FOR_EACH_IMM_USE_FAST since the uses are only being read, not modified
That looks good - usually we use _USE_STMT if we don't want to visit stmts twice
if there are two uses of an SSA name on a stmt. In this particular
case that can't
happen.
Thus 2) is ok, 1) can be done better I think
Thanks,
Richard.
> Does this look OK to commit after bootstrap and regtest on
> x86_64-pc-linux-gnu?
>
> gcc/ChangeLog:
>
> * tree-vrp.c (need_assert_for): Change from bitmap to sbitmap.
> (dump_all_asserts): Adjust accordingly.
> (process_assert_insertions): Likewise.
> (insert_range_assertions): Likewise.
> (register_edge_assert_for_2): Use FOR_EACH_IMM_USE_FAST instead
> of FOR_EACH_IMM_USE_STMT.
>
> Index: gcc/tree-vrp.c
> ===================================================================
> --- gcc/tree-vrp.c (revision 235801)
> +++ gcc/tree-vrp.c (working copy)
> @@ -141,7 +141,7 @@ struct assert_locus
>
> /* If bit I is present, it means that SSA name N_i has a list of
> assertions that should be inserted in the IL. */
> -static bitmap need_assert_for;
> +static sbitmap need_assert_for;
>
> /* Array of locations lists where to insert assertions. ASSERTS_FOR[I]
> holds a list of ASSERT_LOCUS_T nodes that describe where
> @@ -4899,7 +4899,7 @@ void
> dump_all_asserts (FILE *file)
> {
> unsigned i;
> - bitmap_iterator bi;
> + sbitmap_iterator bi;
>
> fprintf (file, "\nASSERT_EXPRs to be inserted\n\n");
> EXECUTE_IF_SET_IN_BITMAP (need_assert_for, 0, i, bi)
> @@ -5248,9 +5248,10 @@ register_edge_assert_for_2 (tree name, edge e, gim
> && TREE_CODE (val) == INTEGER_CST)
> {
> imm_use_iterator ui;
> - gimple *use_stmt;
> - FOR_EACH_IMM_USE_STMT (use_stmt, ui, name)
> + use_operand_p use_p;
> + FOR_EACH_IMM_USE_FAST (use_p, ui, name)
> {
> + gimple *use_stmt = USE_STMT (use_p);
> if (!is_gimple_assign (use_stmt))
> continue;
>
> @@ -6362,7 +6363,7 @@ static void
> process_assert_insertions (void)
> {
> unsigned i;
> - bitmap_iterator bi;
> + sbitmap_iterator bi;
> bool update_edges_p = false;
> int num_asserts = 0;
>
> @@ -6427,7 +6428,8 @@ process_assert_insertions (void)
> static void
> insert_range_assertions (void)
> {
> - need_assert_for = BITMAP_ALLOC (NULL);
> + need_assert_for = sbitmap_alloc (num_ssa_names);
> + bitmap_clear (need_assert_for);
> asserts_for = XCNEWVEC (assert_locus *, num_ssa_names);
>
> calculate_dominance_info (CDI_DOMINATORS);
> @@ -6446,7 +6448,8 @@ insert_range_assertions (void)
> }
>
> free (asserts_for);
> - BITMAP_FREE (need_assert_for);
> + sbitmap_free (need_assert_for);
> + need_assert_for = NULL;
> }
>
> /* Checks one ARRAY_REF in REF, located at LOCUS. Ignores flexible arrays
