This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [lto][patch] Clear DECL_CONTEXT of PARM_DECL


> Exactly.  This is what I expected the patch to fix.  Fixing the inliner and
> unnester to set DECL_CONTEXT properly for PARM_DECLs (those should be the
> only two passes that need to do that).

I can try that, but in general having redundant data is asking for it
to get inconsistent. In gcc that is almost a certainty.

> The patch is changing the semantics of dr_may_alias_p:
>
>> --- a/gcc/tree-data-ref.c
>> +++ b/gcc/tree-data-ref.c
>> @@ -1275,8 +1275,9 @@ dr_may_alias_p (const struct data_reference *a, const struct data_reference *b)
>>        && decl_a && DECL_P (decl_a)
>>        && decl_b && DECL_P (decl_b)
>>        && decl_a != decl_b
>> -      && TREE_CODE (DECL_CONTEXT (decl_a)) == FUNCTION_DECL
>> -      && DECL_CONTEXT (decl_a) == DECL_CONTEXT (decl_b))
>> +      && DECL_CONTEXT (decl_a) == DECL_CONTEXT (decl_b)
>> +      && (!DECL_CONTEXT (decl_a)
>> +         || TREE_CODE (DECL_CONTEXT (decl_a)) == FUNCTION_DECL))
>>      return false;
>
> If DECL_A and DECL_B are globals this function will return false,
> that was not the original intent. Originally, two globals that
> got to this predicate would fail it and 'true' would be returned
> (I don't know whether two globals ever actually reach here,
> though).

It would crash. What the patch changes is that now globals are not the
only thing with DECL_CONTEXT = NULL.

> In looking at the changes to auto_var_in_fn_p, why not just
> return true for PARM_DECLs?  They are locals after all.  This
> change is opening up potential ICEs from find_var_from_fn,
> remap_gimple_stmt and copy_tree_body_r (the call in remap_decls
> should be fine as that only happens on VAR_DECLs).

It would work, but the name of the function would be a bit confusing.
I think the proper answer to the question is this PARM_DECL local to
this function is "I don't know". I could change it to search the
argument list of the function....

> No, but you definitely need to start producing test cases for
> these patches.  If you want, wait until you make the final change
> to DECL_CONTEXT and post the test case for it.  IIRC, your plan
> is to replace DECL_CONTEXT with an indicator that tells whether
> the symbol is local or not, right?

Yes. After reset_decl_lang_specifics. I have no plan to change the FEs.

>> I don't think we should keep information we don't need.
>
> Agreed, but that's not what I'm opposing.  What concerns me here
> is the way the patch was going about it :)
>
>
> Diego.
>


Cheers,
-- 
Rafael Avila de Espindola

Google | Gordon House | Barrow Street | Dublin 4 | Ireland
Registered in Dublin, Ireland | Registration Number: 368047


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]