Hi,
On Mon, Jan 11, 2016 at 05:38:47PM +0100, Jakub Jelinek wrote:
> On Mon, Jan 11, 2016 at 09:41:31AM +0100, Richard Biener wrote:
> > Hum. Can't you check id->remapping_type_depth?
For some reason, last week I reached the conclusion that no. But I
must have done something wrong because I have tested it again today
and just never creating a new decl in remap_decl if
id->remapping_type_depth is non zero is good enough for my testcase
and it survives bootstrap and testing too (previously I thought it did
not).
id->remapping_type_depth seems to be incremented for DECL_VALUE_EXPR
id->as well, so it actually might help in that situation too.
> That said, how do
> > we end up recursing into remap_decl when copying the variable length
> > decl/type? Can't we avoid the recursion (basically avoid remapping
> > variable-size types at all?)
Here I agree with Jakub that there are situations where we have to.
There is a comment towards the end of remap_type_1 saying that when
remapping types, all required decls should have already been mapped.
If that is correct, and I belive it is, the remapping_type_depth test
should be fine.
>
> I guess it depends, VLA types that refer in their various gimplified
> expressions only to decls defined outside of bind stmts we are duplicating
> are fine as is, they don't need remapping, or could be remapped to VLA types
> that use all the same temporary decls.
> VLAs that have some or all references to decls inside of the bind stmts
> we are duplicating IMHO need to be remapped.
> So, perhaps we need to remap_decls in replace_locals_stmt in two phases
> in presence of VLAs (or also vars with DECL_VALUE_EXPR)
I'm a bit worried what would happen do local DECLs that are pointers
to VLAs, because...
> - phase 1 would just walk the
> for (old_var = decls; old_var; old_var = DECL_CHAIN (old_var))
> {
> if (!can_be_nonlocal (old_var, id)
> && ! variably_modified_type_p (TREE_TYPE (old_var), id->src_fn))
...variably_modified_type_p seems to return true for them and...
> remap_decl (old_var, id);
> }
> - phase 2 - do the full remap_decls, but during that arrange that
> remap_decl for non-zero id->remapping_type_depth if (!n) just returns
> decl
...they would not be copied here because remap_decl would not be
duplicating stuff. So I'd end up with an original local decl when I
actually need a duplicate.
But let me go with just checking the remapping_type_depth for now.
Thanks for looking into this,
Martin
> That way, I think if the types refer to some temporaries that are defined
> in the bind stmts being copied, they will be properly duplicated, otherwise
> they will be shared.
> So, we'd need some flag in *id (just bool bitfield would be enough) that would
> allow replace_locals_stmt to set it before the remap_decls call in phase 2
> and clear it afterwards, and use that flag together with
> id->remapping_type_depth in remap_decls.
>
> Jakub
