On Thu, 29 Oct 2015, Tom de Vries wrote: > On 29/10/15 14:12, Richard Biener wrote: > > On Thu, 29 Oct 2015, Tom de Vries wrote: > > > > > >On 29/10/15 12:13, Richard Biener wrote: > > > > > >On Wed, 28 Oct 2015, Tom de Vries wrote: > > > > > > > > > > > > > > > >On 28/10/15 16:35, Richard Biener wrote: > > > > > > > > > > > > > >On Tue, 27 Oct 2015, Tom de Vries wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >On 27/10/15 13:24, Tom de Vries wrote: > > > > > > > > > > > > > > > > > > > > > >Thinking it over a bit more, I > > > > > > > > > > > > realized the constraint > > > > > > > > > > > > > >handling started > > > > > > > > > > > > > > > > > > > > > >to be messy. I've reworked the patch > > > > > > > > > > > > series to simplify that > > > > > > > > > > > > > >first. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 1 Simplify constraint > > > > > > > > > > > > handling > > > > > > > > > > > > > > > > > > > > > > 2 Rename > > > > > > > > > > > > make_restrict_var_constraints to > > > > > > > > > > > > > > > > > > > > > >make_param_constraints > > > > > > > > > > > > > > > > > > > > > > 3 Add recursion to > > > > > > > > > > > > make_param_constraints > > > > > > > > > > > > > > > > > > > > > > 4 Add handle_param > > > > > > > > > > > > parameter to > > > > > > > > > > > > > >create_variable_info_for_1 > > > > > > > > > > > > > > > > > > > > > > 5 Handle recursive > > > > > > > > > > > > restrict pointer in > > > > > > > > > > > > > > > > > > > > > >create_variable_info_for_1 > > > > > > > > > > > > > > > > > > > > > > 6 Handle restrict struct > > > > > > > > > > > > fields recursively > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >Currently doing bootstrap and regtest > > > > > > > > > > > > on x86_64. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >I'll repost the patch series in reply > > > > > > > > > > > > to this message. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >This patch gets rid of this bit of code in > > > > > > > > > > > >intra_create_variable_infos: > > > > > > > > > > > > > > > > > >... > > > > > > > > > > > > > > > > > > if (restrict_pointer_p) > > > > > > > > > > > > > > > > > > make_constraint_from_global_restrict > > > > > > > > > > (p, "PARM_RESTRICT"); > > > > > > > > > > > > > > > > > > else > > > > > > > > > > > > > > > > > >.. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >I already proposed to remove it here ( > > > > > > > > > > > > > > > > > > > > > > > > > > > >https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02426.html > > > > > > > > > > ) but > > > > > > > > > > > >there is a > > > > > > > > > > > > > > > > > >problem with that approach: It can happen > > > > > > > > > > that restrict_pointer_p > > > > > > > > > > > >is true, > > > > > > > > > > > > > > > > > >but > > > > > > > > > > > > > > > > > >p->only_restrict_pointers is false. This > > > > > > > > > > happens with fipa-pta, > > > > > > > > > > > >when > > > > > > > > > > > > > > > > > >create_function_info_for created a varinfo > > > > > > > > > > for the parameter > > > > > > > > > > > >before > > > > > > > > > > > > > > > > > >intra_create_variable_infos was called. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >The patch handles that case now by setting > > > > > > > > > > > >p->only_restrict_pointers. > > > > > > > > > > > > > > > > > > > > > > > > > > > >Hmm, but ... restrict only has an effect in non-IPA > > > > > > > > mode. > > > > > > > > > > > > > > > > > > > >Indeed, I also realized that by now. > > > > > > > > > > > > > > > > > > > >I wrote attached patch to make that explicit and simplify > > > > > > fipa-pta. > > > > > > > > > > > > > > > > > > > >OK for trunk if bootstrap and reg-test succeeds? > > > > > > > >First, there was an error in the patch, it tested for flag_ipa_pta (so it > > > also > > > >affected ealias), but it was supposed to test for in_ipa mode. That is > > > fixed > > > >in attached version. > > > > > > > > > >I don't see the patch simplifies anything but only removes spurious > > > > > >settings by adding IMHO redundant checks. > > > > > > > >Consider testcase: > > > >... > > > >int __attribute__((noinline, noclone)) > > > >foo (int *__restrict__ a, int *__restrict__ b) > > > >{ > > > > *a = 1; > > > > *b = 2; > > > >} > > > > > > > >int __attribute__((noinline, noclone)) > > > >bar (int *a, int *b) > > > >{ > > > > foo (a, b); > > > >} > > > >... > > > > > > > >The impact of this patch in the pta dump (focusing on the constraints > > > bit) is: > > > >... > > > > Generating constraints for foo (foo) > > > > > > > >-foo.arg0 = &PARM_NOALIAS(20) > > > >-PARM_NOALIAS(20) = NONLOCAL > > > >-foo.arg1 = &PARM_NOALIAS(21) > > > >-PARM_NOALIAS(21) = NONLOCAL > > > >+foo.arg0 = &NONLOCAL > > > >+foo.arg1 = &NONLOCAL > > > >... > > > > > > > >That's the kind of simplifications I'm trying to achieve. > > Yes, but as I said we should refactor things to avoid calling > > the intra constraints generation from the IPA path. > > Ah, I see. > > So, like this? OK for trunk if bootstrap and reg-test succeeds?
Yes, like this. But you miss to apply the same to the static chain, and the varargs "rest". Ok with that change. Richard.