On Fri, Jan 16, 2015 at 11:51 AM, Ilya Enkovich <enkovich....@gmail.com> wrote:
> On 14 Jan 19:40, Richard Biener wrote:
>> On January 14, 2015 5:23:21 PM CET, Ilya Enkovich <enkovich....@gmail.com>
>> wrote:
>> >On 14 Jan 15:35, Richard Biener wrote:
>> >> On Wed, Jan 14, 2015 at 3:28 PM, Ilya Enkovich
>> ><enkovich....@gmail.com> wrote:
>> >> > Hi,
>> >> >
>> >> > SRA gimple passes may add loads to functions with no SSA update.
>> >Later it causes ICE when function with not updated SSA is processed by
>> >gimple passes. This patch fixes it by calling update_ssa.
>> >> >
>> >> > Bootstrapped and checked on x86_64-unknown-linux-gnu. OK for
>> >trunk?
>> >>
>> >> No. I have removed this quadratic update-ssa call previously. It
>> >should
>> >> simply keep SSA for up-to-date manually (see how it does
>> >gimple_set_vuse
>> >> in some cases, probably not all required ones?).
>> >>
>> >
>> >Would it be OK to call update_ssa only in case we don't have a proper
>> >VUSE for call?
>>
>> No, and most definitely not here.
>>
>>
>> Are we allowed to just emit error due to incorrect
>> >attribute?
>>
>> No, I don't think so either. But we may drop it.
>>
>> Richard.
>>
>
> Here is a version with SRA disabled for functions with __attribute__((const))
> as you suggested in tracker. Is it OK?
>
> Bootstrapped and checked on x86_64-unknown-linux-gnu.
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2015-01-16 Ilya Enkovich <ilya.enkov...@intel.com>
>
> PR middle-end/64353
> * tree-sra.c (ipa_sra_preliminary_function_checks): Reject
> functions with const attribute.
>
> gcc/testsuite/
>
> 2015-01-16 Ilya Enkovich <ilya.enkov...@intel.com>
>
> PR middle-end/64353
> * g++.dg/pr64353.C: New.
>
>
> diff --git a/gcc/testsuite/g++.dg/pr64353.C b/gcc/testsuite/g++.dg/pr64353.C
> new file mode 100644
> index 0000000..7859918
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/pr64353.C
> @@ -0,0 +1,15 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +class C
> +{
> + int y, x;
> + void i ();
> + bool __attribute__((const)) xx () { return x; }
> +};
> +
> +void C::i ()
> +{
> + if (xx ())
> + x = 1;
> +}
> diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
> index f560fe0..f24ca9f 100644
> --- a/gcc/tree-sra.c
> +++ b/gcc/tree-sra.c
> @@ -5015,6 +5015,13 @@ ipa_sra_preliminary_function_checks (struct
> cgraph_node *node)
> return false;
> }
>
> + if (lookup_attribute ("const", DECL_ATTRIBUTES (node->decl)))
TREE_READONLY (node->decl)
but I'm now not sure that we can really trust this given that only fixup_cfg
will eventually fixup stmts in callers after ipa-pure-const ran. The above
also doesn't handle "no vops" functions.
I think a better place to check this would be inside
some_callers_have_mismatched_arguments_p where you'd check
|| !gimple_vuse (cs->call_stmt)
of course the reasoning printed is wrong then.
We can fix this by running update-ssa in fixup_cfg as well, and I guess
I like that more.
Index: tree-cfg.c
===================================================================
--- tree-cfg.c (revision 219714)
+++ tree-cfg.c (working copy)
@@ -8754,7 +8804,7 @@ const pass_data pass_data_fixup_cfg =
PROP_cfg, /* properties_required */
0, /* properties_provided */
0, /* properties_destroyed */
- 0, /* todo_flags_start */
+ TODO_update_ssa_only_virtuals, /* todo_flags_start */
0, /* todo_flags_finish */
};
this variant is pre-approved if it passes bootstrap and regtest.
Thanks,
Richard.
> + {
> + if (dump_file)
> + fprintf (dump_file, "Function has const attribute.\n");
> + return false;
> + }
> +
> if (!tree_versionable_function_p (node->decl))
> {
> if (dump_file)
