On Mon, Oct 29, 2012 at 05:10:10PM +0100, Richard Biener wrote:
> On Mon, Oct 29, 2012 at 4:25 PM, Dehao Chen <de...@google.com> wrote:
> > On Mon, Oct 29, 2012 at 7:17 AM, Michael Matz <m...@suse.de> wrote:
> >> Hi,
> >>
> >> On Mon, 29 Oct 2012, Richard Biener wrote:
> >>
> >>> > Well, you merely moved the bogus code to gimple-low.c. It is bogus
> >>> > because you unconditionally overwrite TREE_BLOCK of all operands (and
> >>> > all
> >
> > Emm, then in gimple-low.c, we should probably not unconditionally
> > overwrite gimple_block for stmts too?
>
> gimple stmts have no block before gimple-low.
And tree expressions don't have TREE_BLOCK before gimple-low either.
So IMNSHO it is gimple-low.c that should set TREE_BLOCK of all the gimple
stmts as well as all expression in the operands. It is not overwriting
anything, no frontend sets TREE_BLOCK for any expression, the way frontends
associate IL with BLOCKs is by putting them inside of BIND_EXPR/GIMPLE_BIND
after gimplification, and it is gimple-low responsibility to set it.
In 4.3 before tuples, it was solely gimple-low that set TREE_BLOCK
initially. Before the location_t changes, again it was gimple-low that
was the first setter of TREE_BLOCK, which was valid for all
IS_EXPR_CODE_CLASS. So, IMNSHO gimple-low should merge location_t
with block for all gimple stmts and all tree expressions used in its
operands. It shouldn't be set on trees that can be shared, so
say decls etc. should keep using just location_t's without associated block.
So perhaps the right test for gimple-low setting of block is
CAN_HAVE_LOCATION_P (exp) && !tree_node_can_be_shared (exp).
Jakub
