This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Address lowering [1/3] Main patch
- From: "William J. Schmidt" <wschmidt at linux dot vnet dot ibm dot com>
- To: Michael Matz <matz at suse dot de>
- Cc: Richard Guenther <richard dot guenther at gmail dot com>, gcc-patches at gcc dot gnu dot org, bergner at vnet dot ibm dot com
- Date: Tue, 05 Jul 2011 09:05:27 -0500
- Subject: Re: [PATCH] Address lowering [1/3] Main patch
- References: <Pine.LNX.4.64.1107041713120.17483@wotan.suse.de>
On Mon, 2011-07-04 at 17:30 +0200, Michael Matz wrote:
> Hi,
>
> On Mon, 4 Jul 2011, Richard Guenther wrote:
>
> > I still do not like the implementation of yet another CSE machinery
> > given that we already have two.
>
> From reading it it really seems to be a normal block-local CSE, without
> anything fancy. Hence, moving the pass just a little earlier (before
> pass_vrp/pass_dominator) should already provide for all optimizations. If
> not those should be improved.
>
> I see that it is used for also getting rid of the zero-offset statements
> in case non-zero-offsets follow. I think that's generally worthwhile so
> probably should be done in one of the above optimizers.
OK, thanks for the suggestion. I can certainly look into this.
>
> You handle NOP_EXPR different from CONVERT_EXPR. The middle-end doesn't
> distinguish between them (yes, ignore the comment about one generating
> code, the other not).
>
Ah, thanks, good to know.
> Your check for small types:
>
> + if (TYPE_MODE (TREE_TYPE (TREE_OPERAND (expr, 0))) == SImode)
> + ref_found = true;
>
> You probably want != BLKmode .
>
OK.
> + if (changed && is_zero_offset_ref (gimple_assign_lhs (stmt)))
> + VEC_safe_push (gimple, heap, zero_offset_refs, stmt);
> +
> + rhs1 = gimple_assign_rhs1_ptr (stmt);
> + rhs_changed = tree_ssa_lower_addr_tree (rhs1, gsi, speed, false);
> +
> + /* Record zero-offset mem_refs on the RHS. */
> + if (rhs_changed && is_zero_offset_ref (gimple_assign_rhs1 (stmt)))
> + VEC_safe_push (gimple, heap, zero_offset_refs, stmt);
>
> This possibly adds stmt twice to zero_offset_refs. Do you really want
> this?
>
Hm, I didn't think it was (currently) possible for a gimple statement to
have a mem-ref on both RHS and LHS. Is that incorrect? This is easily
changed if so, or if the possibility should be left open for the future.
>
> Ciao,
> Michael.
Thanks,
Bill