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.
>>> > operands operands) with the statements block. I assume the unit-test you
>>> > added shows the problem you were trying to fix?
>>> >
>>> > From the scan-assembler-no directive I'm not really sure what exactly the
>>> > problem is you're seeing. Can you try to describe it with words for the
>>> > example code? Which operands has no tree-block where it should have one,
>>> > or which one has the wrong tree-block?
>>>
>>> trees without block could be an issue when we extract them to some other
>>> statement (then without block), and move that to some random place.
>>
>> Even then it would be either the frontends job to set the block, or the
>> the job of the pass that introduced the new expression, when there is a
>> clearly sane candidate for the block.
>
> Please correct me if I'm wrong: front-end does not set blocks for
> either stmt/expr. lower_stmt is the first place that block is set for
> stmt, thus I think it should also be the first place to set block for
> expr.
No, only the block on the gimple stmt matters for gimple. You should not
need to touch the operands block.
>>
>>> But the only issue should be that the stmt/expressions effective block
>>> becomes a different one (the currently "active" one during expansion).
>
> I agree. Initially, both stmt and expressions is set to NULL block.
> Whenever we update the stmt's block, we should also update the
> expression's block, otherwise they will be inconsistent.
I don't think so.
>>
>> Yep.
>>
>>> I don't see how we could end up generating too many block location
>>> DIEs because of this.
>
> For the unittest I added, all the assembly code guarded by "if" are
> should be within one lexical block, which is lock:
>
> LBB1
> {
> asm1
> asm2
> asm3
> asm4
> ....
> }
>
> However, because some expression are with NULL lexical block, these
> assembly codes are separated by several discontinual lexical block
> which is like:
>
> LBB1
> {
> asm1
> }
> asm2
asm2 should have gone into the currently open scope (similiar for what
we do for line number information).
> LBB2
> {
> asm3
> }
> asm4
> ....
>
>>
>> And even if, I don't see what's the _problem_ with too many block
>> locations, except bloat.
>
> Yes, bloat is the major problem.
>
> Thanks,
> Dehao
>
>>
>>
>> Ciao,
>> Michael.
