Hi, Thanks for your review. Here comes the 2nd version patch modified according to your comments. Is it ok? Also could you please commit it if ok because I have no write access?
The new patch is tested against x86-linux-gnu.
Thanks.
2011-11-15 Bin Cheng <bin.ch...@arm.com>
PR rtl-optimization/50663
* cprop.c (implicit_set_indexes): New global variable.
(insert_set_in_table): Add additional parameter, record implicit set
info.
(hash_scan_set): Add additional parameter.
(compute_hash_table_work): And
(hash_scan_insn): Pass implicit to hash_scan_set.
(compute_cprop_data): Add implicit set to AVIN of block which the
implicit set is recorded for.
(one_cprop_pass): Handle implicit_set_indexes array.
> -----Original Message-----
> From: Eric Botcazou [mailto:ebotca...@adacore.com]
> Sent: Saturday, November 12, 2011 12:35 AM
> To: Bin Cheng
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH RFA] rtl-optimization/PR50663, conditional propagation
> missed in cprop.c pass
>
> > 2011-11-07 Bin Cheng <bin.ch...@arm.com>
> >
> > PR rtl-optimization/50663
> > * cprop.c (bb_implicit): New global variable.
> > (insert_set_in_table): Add additional parameter, record implicit set
> > info.
> > (hash_scan_set): Add additional parameter.
> > (compute_hash_table_work): And
> > (hash_scan_insn): Pass implicit to hash_scan_set.
> > (compute_cprop_data): Add implicit set to AVIN of block which the
> > implicit set is recorded for.
> > (one_cprop_pass): Handle bb_implicit array.
>
> [80 columns at most for ChangeLog entries as well]
>
> The patch is OK with the following changes:
>
> @@ -116,6 +116,10 @@
> /* Array of implicit set patterns indexed by basic block index. */
> static rtx *implicit_sets;
>
> +/* Array of bitmap_index of corresponding implicit set, indexed by
> + basic block index. */
> +static int *bb_implicit;
>
> A better name is implicit_set_indexes:
>
> /* Array of indexes of expressions for implicit set patterns indexed by
basic
> block index. In other words, implicit_set_indexes[i] is the
bitmap_index
> of the expression whose RTX is implicit_sets[i]. */
> static int *implicit_set_indexes;
>
>
> + /* Record bitmap_index of the implicit set in bb_implicit. */
> + if (implicit)
> + bb_implicit[BLOCK_FOR_INSN(cur_occr->insn)->index] =
> + cur_expr->bitmap_index;
>
> cur_occr->insn is just insn.
>
>
> + /* Merge implicit set into CPROP_AVIN. There are always
> + available at the entry of corresponding basic block. */
>
> "...implicit sets into CPROP_AVIN. They are..."
>
> + FOR_EACH_BB (bb)
> + {
> + int index = bb_implicit[bb->index];
> + if (index != -1)
> + SET_BIT (cprop_avin[bb->index], (unsigned int)index);
>
> The cast is superfluous.
>
> I think that an explanation as to why we need to do this is in order
(after
> all, this went unnoticed until now) along the lines of: "We need to do
this
> because 1) implicit sets aren't recorded for the local pass so they cannot
> be propagated within their basic block by this pass and 2) the global pass
> would otherwise propagate them only in the successors of their basic
block."
>
> Btw, you'll need to slightly adjust the patch because of my changes to
cprop.c.
>
> Thanks for investigating and addressing this issue.
>
> --
> Eric Botcazou
pr50663-20111116.patch
Description: Binary data
