,.,. C974001 ACATS 2.5 06-01-10 12:54:17 ---- C974001 Asynchronous Select: Trigger is delay_relative which completes before abortable part. (killed) ,.,. C974013 ACATS 2.5 06-01-10 13:57:39 ---- C974013 Asynchronous Select: Trigger is delay_until which completes before abortable part. (killed)
Confirmed on x86. It's a miscompilation of the runtime since they do terminate if struct aliasing is enabled but the original runtime is used.
s-taasde.adb:Time_Enqueue is miscompiled at -O2, more specifically: Q := Timer_Queue.Succ; while Q.Resume_Time < T loop Q := Q.Succ; end loop; -- Q is the block that has Resume_Time equal to or greater than -- T. After the insertion we want Q to be the successor of D. D.Succ := Q; D.Pred := Q.Pred; D.Pred.Succ := D; Q.Pred := D; -- If the new element became the head of the queue, -- signal the Timer_Server to wake up. if Timer_Queue.Succ = D then Timer_Attention := True; STPO.Wakeup (Timer_Server_ID, ST.Timer_Server_Sleep); end if; FRE thinks that it can propagate the value Timer_Queue.Succ has at the top of the code down to the conditional of the test at the bottom. Of course that's not true since the assignment game in the middle can clobber it. I'm further digging but does this ring a bell to you, Richard? TIA.
> I'm further digging but does this ring a bell to you, Richard? I guess we should have a few V_MAY_DEFs for SFT.115, right? # VUSE <SFT.115_55>; q_17 = system__tasking__async_delays__timer_queue.succ; # q_1 = PHI <q_17(4), q_27(6)>; <L2>:; # VUSE <SMT.120_56>; D.1231_18 = q_1->resume_time; if (D.1231_18 >= t_13) goto <L4>; else goto <L3>; <L3>:; # VUSE <SMT.120_56>; q_26 = q_1->succ; q_27 = q_26; goto <bb 5> (<L2>); <L4>:; # SMT.120_58 = V_MAY_DEF <SMT.120_56>; d_12->succ = q_1; # VUSE <SMT.120_58>; D.1233_19 = q_1->pred; # SMT.120_59 = V_MAY_DEF <SMT.120_58>; d_12->pred = D.1233_19; # VUSE <SMT.120_59>; D.1234_20 = d_12->pred; # SMT.120_60 = V_MAY_DEF <SMT.120_59>; D.1234_20->succ = d_12; # SMT.120_61 = V_MAY_DEF <SMT.120_60>; q_1->pred = d_12; # VUSE <SFT.115_55>; D.1235_21 = system__tasking__async_delays__timer_queue.succ; if (D.1235_21 == d_12) goto <L5>; else goto <L6>;
Hum... perhaps I'm naive, but what's the point of creating SFTs for addressable variables? What's the mechanism that is supposed to add V_MAY_DEFs for SFTs of such variables when V_MAY_DEFs are added for the variables?
Danny might have an idea and/or time to look at this.
I guess (only guess!) that the types differe in a way that aliasing does not see them clobbered by means of type based alias analysis. Can you try if -fno-strict-aliasing fixes it?
> I guess (only guess!) that the types differe in a way that aliasing does not > see them clobbered by means of type based alias analysis. Can you try if > -fno-strict-aliasing fixes it? No, there is really a hole somewhere, reduced C testcase to be attached. When structure aliasing is enabled, the SMT for the structure type doesn't alias the structure, which seems wrong to me since the structure is marked as addressable. Daniel, could you take a quick look? We can't enable structure aliasing for Ada because of this problem in the aliasing machinery. Thanks in advance.
Created attachment 11103 [details] Reduced C testcase. Compile at -O1 on x86.
Yay. Reduced testcase: extern void abort (void); struct delay_block { struct delay_block *succ; }; static struct delay_block Timer_Queue; struct delay_block* time_enqueue (struct delay_block *d) { struct delay_block *q = Timer_Queue.succ; d->succ = (void *)0; return Timer_Queue.succ; } int main(void) { Timer_Queue.succ = &Timer_Queue; if (time_enqueue (&Timer_Queue) != (void*)0) abort (); return 0; } Pointed-to sets for pointers in time_enqueue d_3, is dereferenced, its value escapes, points-to anything SFT.0_1 q_2 D.1529_4, its value escapes <retval>_5 # VUSE <SFT.0_1>; q_2 = Timer_Queue.succ; # SMT.5_7 = V_MAY_DEF <SMT.5_6>; d_3->succ = 0B; # VUSE <SFT.0_1>; D.1529_4 = Timer_Queue.succ;
This shows a bunch of bugs actually: PTA thinks it doesn't need to solve the graph when it does. The subvar isn't initially marked as addressable if the regular var is in create_sft. The second is what is actually causing your bug, AFAICT. In create_sft, add TREE_ADDRESSABLE (sft) = TREE_ADDRESSABLE (var) in all the flags it is copying, and it should fix the bug. I am bootstapping this change right now.
Subject: Re: [4.1/4.2 Regression] ACATS tests c974001 and c974013 do not terminate with struct aliasing enabled On Thu, 23 Mar 2006, dberlin at gcc dot gnu dot org wrote: > This shows a bunch of bugs actually: > > PTA thinks it doesn't need to solve the graph when it does. > The subvar isn't initially marked as addressable if the regular var is in > create_sft. > > The second is what is actually causing your bug, AFAICT. > > In create_sft, add TREE_ADDRESSABLE (sft) = TREE_ADDRESSABLE (var) in all the > flags it is copying, and it should fix the bug. > I am bootstapping this change right now. Yep, that fixes it. Richard.
> PTA thinks it doesn't need to solve the graph when it does. > The subvar isn't initially marked as addressable if the regular var is in > create_sft. > > The second is what is actually causing your bug, AFAICT. > > In create_sft, add TREE_ADDRESSABLE (sft) = TREE_ADDRESSABLE (var) in all the > flags it is copying, and it should fix the bug. That is indeed sufficient to fix the bug for the C testcase but not for an equivalent Ada testcase, so Richard might have been right in thinking that there is also some type frobbing on the Ada side :-( > I am bootstapping this change right now. Thanks. Do not forget to put PR tree-optimization/25737 in the ChangeLog.
> That is indeed sufficient to fix the bug for the C testcase but not for an > equivalent Ada testcase, so Richard might have been right in thinking that > there is also some type frobbing on the Ada side :-( Not a bug, but a feature. :-) Everything is not addressable in Ada, unlike in C. The tree aliasing machinery appears to totally overlook the flag /* Used in a FIELD_DECL to indicate that we cannot form the address of this component. */ #define DECL_NONADDRESSABLE_P(NODE) \ (FIELD_DECL_CHECK (NODE)->decl_common.decl_flag_3)
The Ada testcase is correctly compiled at -O with the create_sft change and Index: tree-ssa-structalias.c =================================================================== --- tree-ssa-structalias.c (revision 112080) +++ tree-ssa-structalias.c (working copy) @@ -3661,7 +3661,7 @@ push_fields_onto_fieldstack (tree type, } for (field = TYPE_FIELDS (type); field; field = TREE_CHAIN (field)) - if (TREE_CODE (field) == FIELD_DECL) + if (TREE_CODE (field) == FIELD_DECL && ! DECL_NONADDRESSABLE_P (field)) { bool push = false; int pushed = 0;
> The tree aliasing machinery appears to totally overlook the flag > > /* Used in a FIELD_DECL to indicate that we cannot form the address of > this component. */ > #define DECL_NONADDRESSABLE_P(NODE) \ > (FIELD_DECL_CHECK (NODE)->decl_common.decl_flag_3) Another one. :-) /* For an ARRAY_TYPE, indicates that it is not permitted to take the address of a component of the type. */ #define TYPE_NONALIASED_COMPONENT(NODE) \ (ARRAY_TYPE_CHECK (NODE)->type.transparent_union_flag)
Subject: Re: [4.1/4.2 Regression] ACATS tests c974001 and c974013 do not terminate with struct aliasing On Thu, 2006-03-23 at 18:24 +0000, ebotcazou at gcc dot gnu dot org wrote: > > ------- Comment #15 from ebotcazou at gcc dot gnu dot org 2006-03-23 18:24 ------- > > The tree aliasing machinery appears to totally overlook the flag > > > > /* Used in a FIELD_DECL to indicate that we cannot form the address of > > this component. */ > > #define DECL_NONADDRESSABLE_P(NODE) \ > > (FIELD_DECL_CHECK (NODE)->decl_common.decl_flag_3) > > Another one. :-) > > /* For an ARRAY_TYPE, indicates that it is not permitted to > take the address of a component of the type. */ > #define TYPE_NONALIASED_COMPONENT(NODE) \ > (ARRAY_TYPE_CHECK (NODE)->type.transparent_union_flag) Errr, but that would make it non-addressable, and thus, non-aliasable, which is the exact opposite effect of what is causing the problem. > >
> Errr, but that would make it non-addressable, and thus, non-aliasable, > which is the exact opposite effect of what is causing the problem. Take a look at alias.c:record_component_aliases. :-)
I don't get it either. Comment #14 makes us not create an SFT for the structure element, which of course works around the existing problem as -fno-tree-salias does. How can the addressability of Succ make a difference for the testcase? We only take the addresses of type TimerQueue.
> How can the addressability of Succ make a difference for the testcase? How do we determine that the SMT of the structure type and the SFT alias?
Subject: Re: [4.1/4.2 Regression] ACATS tests c974001 and c974013 do not terminate with struct aliasing On Thu, 2006-03-23 at 22:21 +0000, ebotcazou at gcc dot gnu dot org wrote: > > ------- Comment #17 from ebotcazou at gcc dot gnu dot org 2006-03-23 22:21 ------- > > Errr, but that would make it non-addressable, and thus, non-aliasable, > > which is the exact opposite effect of what is causing the problem. > > Take a look at alias.c:record_component_aliases. :-) Yeah, and that has nothing to do with the problem.. Really. You think it's going to make it use the parent's alias set and make, and this may be true for type based aliasing. However, either the documentation for the flag is completely wrong, or you could just give these things their own alias set if it's not addressable, since no pointer can alias it. That is what it means to be non-addressable. If nothing can take the address, then it can't be pointed to. It does not mean "it's really still addressable through some parent pointer magic blah blah blah". At least, in *any other definition we use in the compiler". If you can get a pointer to it, then by the definition you "an address of the component can be formed", and htus, it would not be DECL_NONADDRESSABLE_P. Thus, making PTA using this flag would make your problem *worse*, not better.
Subject: Re: [4.1/4.2 Regression] ACATS tests c974001 and c974013 do not terminate with struct aliasing On Thu, 2006-03-23 at 22:33 +0000, ebotcazou at gcc dot gnu dot org wrote: > > ------- Comment #19 from ebotcazou at gcc dot gnu dot org 2006-03-23 22:33 ------- > > How can the addressability of Succ make a difference for the testcase? > > How do we determine that the SMT of the structure type and the SFT alias? It depends. In the case i've fixed, when we don't have a valid points-to set (which is the case here), we walk the *addressable* variables, and see if they alias with the other *addresable* variables. These addressable variables include the SFT's with TREE_ADDRESSABLE set. However, it is non-sensible for an SFT of a DECL_NONADDRESSABLE_P field to have TREE_ADDRESSABLE set, since it can't be addressed. That said *do not* currently make use of this DECL_NONADRESSABLE_P information, so this cannot be the problem, as it will be marked TREE_ADDRESSABLE if it is addressed in some way, and that flag will not be cleared as long as something is taking what appears to be the address of it.
It looks like there was a C testcase, which has now been fixed; is there still an issue for languages other than Ada?
> It looks like there was a C testcase, which has now been fixed; is there still > an issue for languages other than Ada? I presume you meant is there still an issue for languages where everything is addressable? If so, very likely not. Now, for other languages, Daniel's patch has made things worse. :-)
I'm sorry to hear things have gotten worse for Ada, but I'm none-the-less downgrading this to P5, as there is no longer any evidence this affects our primary languages. (I've not fully understood the discussion here, but if everything in Ada is addressable, then it seems to me that everything should have TREE_ADDRESSABLE set.)
> (I've not fully understood the discussion here, but if everything in Ada is > addressable, then it seems to me that everything should have TREE_ADDRESSABLE > set.) Grumpf... you read a little too quickly, that is exactly the opposite. The struct aliasing machinery essentially assumes that everything is addressable (modulo bit-fields) whereas *nothing* is addressable by default in Ada.
Subject: Re: [4.1/4.2 Regression] ACATS c974001 c974013 hang with struct aliasing ebotcazou at gcc dot gnu dot org wrote: > ------- Comment #25 from ebotcazou at gcc dot gnu dot org 2006-05-15 05:36 ------- >> (I've not fully understood the discussion here, but if everything in Ada is >> addressable, then it seems to me that everything should have TREE_ADDRESSABLE >> set.) > > Grumpf... you read a little too quickly, that is exactly the opposite. The > struct aliasing machinery essentially assumes that everything is addressable > (modulo bit-fields) whereas *nothing* is addressable by default in Ada. > Read comment 20 again. If nothing was addressable, and we relied on this, your problem would get worse, *not better*. It will cause *strictly more* things to be marked unaliased, and your problem is that this really is aliasable.
Will not be fixed in 4.1.1; adjust target milestone to 4.1.2.
See hang on hppa1.1-hp-hpux10.20 for c974013. I don't know whether these are the same problem but c974004 and c974005 are also failing on this target: ,.,. C974004 ACATS 2.5 06-06-11 23:43:09 ---- C974004 Asynchronous Select: Trigger is queued on a task entry and is completed first by an exception. raised C974004_0.PROPAGATED_FROM_TASK : s-tasini.adb:286 ,.,. C974005 ACATS 2.5 06-06-11 23:44:16 ---- C974005 ATC: trigger is queued but task terminates before call is serviced. raised TASKING_ERROR : s-tasini.adb:286
On s390x c974001, c974013 and cb20001 run into a infinite loop with current mainline. At least the first two look related - not sure about the third.
All three (c974001, c974013 and cb20001) do not fail on s390x anymore since my patch removing the CLOBBERs for eh registers has been applied: http://gcc.gnu.org/ml/gcc-patches/2006-09/msg00498.html If the analysis in this PR is correct it seems unlikely to me that the patch is directly related to the cause of the problem. Maybe it simply hides the real problem somehow.
Note that Ada requires a patch to enable struct-aliasing, as it does bool gnat_post_options (const char **pfilename ATTRIBUTE_UNUSED) { flag_inline_trees = 1; if (!flag_no_inline) flag_no_inline = 1; if (flag_inline_functions) flag_inline_trees = 2; flag_tree_salias = 0; return false; } at the moment.
> Note that Ada requires a patch to enable struct-aliasing, as it does Right, there is some confusion in this PR. The problem does *not* exist with the pristine mainline sources so I'm reclassifying.
Ok. I wasn't aware of this. On s390 these testcases failed even without the struct-aliasing patch. So obviously it was just a coincidence that the same testcases failed on s390 (plus one more). Sorry if I made the confusion complete.
Subject: Re: ACATS c974001 c974013 hang with struct aliasing On 21 Sep 2006 14:49:14 -0000, krebbel at gcc dot gnu dot org <gcc-bugzilla@gcc.gnu.org> wrote: > > > ------- Comment #33 from krebbel at gcc dot gnu dot org 2006-09-21 14:49 ------- > Ok. I wasn't aware of this. On s390 these testcases failed even without the > struct-aliasing patch. So obviously it was just a coincidence that the same > testcases failed on s390 (plus one more). > Sorry if I made the confusion complete. It's okay, since this bug will never be fixed until someone can explain why they believe making things non-addressable will result in more aliasing, and not less. I suspect the reality is there is some fundamental misunderstanding of how aliasing or addressability works on the part of the Ada folks (no offense to them, it's just the arguments they keep offering are, well, strange to anyone who understands aliasing).
> I suspect the reality is there is some fundamental misunderstanding of > how aliasing or addressability works on the part of the Ada folks (no > offense to them, it's just the arguments they keep offering are, well, > strange to anyone who understands aliasing). I suspect that we were simply not on the same page, you being at a general level about aliasing/addressability and I being at the implementation level about DECL_NONADDRESSABLE_P and TYPE_NONALIASED_COMPONENT. As I told Richard in Ottawa, I plan to revisit this for 4.3.
Subject: Re: ACATS c974001 c974013 hang with struct aliasing On 21 Sep 2006 16:53:49 -0000, ebotcazou at gcc dot gnu dot org <gcc-bugzilla@gcc.gnu.org> wrote: > > > ------- Comment #35 from ebotcazou at gcc dot gnu dot org 2006-09-21 16:53 ------- > > I suspect the reality is there is some fundamental misunderstanding of > > how aliasing or addressability works on the part of the Ada folks (no > > offense to them, it's just the arguments they keep offering are, well, > > strange to anyone who understands aliasing). > > I suspect that we were simply not on the same page, you being at a general > level about aliasing/addressability and I being at the implementation level > about DECL_NONADDRESSABLE_P and TYPE_NONALIASED_COMPONENT. > No, really, you don't seem to understand. If you respect these DECL_NONADDRESSABLE_P or TYPE_NONALIASED_COMPONENT flags, you are going to make them unaliased. Your whole bug report is that they are not aliased and should be, and that the loads and stores currently don't interfere but should. This is what everyone has told you so far. I've copied Diego, and I expect he will tell you the same thing. Diego, the short summary is that Eric has some Ada testcases where we end up with less V_MAY_DEF's than he thinks should. He believes that respecting these flags, which specify you cannot form the address of a certain component, etc, will somehow cause him to end up with more aliasing and fix his testcase by anything other than luck.
> No, really, you don't seem to understand. > If you respect these DECL_NONADDRESSABLE_P or > TYPE_NONALIASED_COMPONENT flags, you are going to make them unaliased. > Your whole bug report is that they are not aliased and should be, and > that the loads and stores currently don't interfere but should. I think I understand your viewpoint: the name of TYPE_NONALIASED_COMPONENT and DECL_NONADDRESSABLE_P seems to imply than setting them would always result in less V_MAY_DEF's in the code. But... > Diego, the short summary is that Eric has some Ada testcases where we > end up with less V_MAY_DEF's than he thinks should. He believes that > respecting these flags, which specify you cannot form the address of a > certain component, etc, will somehow cause him to end up with more > aliasing and fix his testcase by anything other than luck. ...that's not so simple. If you look at how these flags work in GCC 3.x, you'll see that setting them has some impact on the alias sets used to access memory references, via can_address_p and the MEM_KEEP_ALIAS_SET_P flag. In GCC 4 dialect, this would result in different V_MAY_DEF's, not less. I'm not saying that this is a sane design or that we should try to replicate it in GCC 4, I'm just saying that for the time being struct aliasing totally overlooks this mechanism and doesn't work for Ada because of that.
Subject: Re: ACATS c974001 c974013 hang with struct aliasing On 24 Sep 2006 18:23:41 -0000, ebotcazou at gcc dot gnu dot org <gcc-bugzilla@gcc.gnu.org> wrote: > > > ------- Comment #37 from ebotcazou at gcc dot gnu dot org 2006-09-24 18:23 ------- > > No, really, you don't seem to understand. > > If you respect these DECL_NONADDRESSABLE_P or > > TYPE_NONALIASED_COMPONENT flags, you are going to make them unaliased. > > Your whole bug report is that they are not aliased and should be, and > > that the loads and stores currently don't interfere but should. > > I think I understand your viewpoint: the name of TYPE_NONALIASED_COMPONENT > and DECL_NONADDRESSABLE_P seems to imply than setting them would always > result in less V_MAY_DEF's in the code. But... > The name, and all the documentation, which say they cannot be addressed, which means they cannot be pointed to by any pointer, which means they are unaliased. > > Diego, the short summary is that Eric has some Ada testcases where we > > end up with less V_MAY_DEF's than he thinks should. He believes that > > respecting these flags, which specify you cannot form the address of a > > certain component, etc, will somehow cause him to end up with more > > aliasing and fix his testcase by anything other than luck. > > ...that's not so simple. If you look at how these flags work in GCC 3.x, > you'll see that setting them has some impact on the alias sets used to access > memory references, via can_address_p and the MEM_KEEP_ALIAS_SET_P flag. > In GCC 4 dialect, this would result in different V_MAY_DEF's, not less. If so, then you've both hacked around something more funamental, and the documentation of all these flags don't actually match what you really mean. > > I'm not saying that this is a sane design or that we should try to replicate > it in GCC 4, I'm just saying that for the time being struct aliasing totally > overlooks this mechanism and doesn't work for Ada because of that. Okay, and i'm saying i don't plan on accepting fixes that appear to hack around well accepted infrastructure to try to fix symptoms. Really. That's all. I'm not going to approve patches that randomly skip fields because it seems to get the right result sometimes. If you want to try to explain what all this is actually trying to do, i'm happy to work with you to come up with a sane solution.
(In reply to comment #38) > The name, and all the documentation, which say they cannot be > addressed, which means they cannot be pointed to by any pointer, which > means they are unaliased. > Absolutely. If the Ada FE somehow wants these two predicates to generate *more* VDEF/VUSE chains, then there is something fundamentally wrong with it. The meaning of these two predicates is the exact opposite, they will lead to *fewer* VDEF/VUSE chains. > > I'm not saying that this is a sane design or that we should try to replicate > > it in GCC 4, I'm just saying that for the time being struct aliasing totally > > overlooks this mechanism and doesn't work for Ada because of that. > Okay, and i'm saying i don't plan on accepting fixes that appear to > hack around well accepted infrastructure to try to fix symptoms. > Really. That's all. I'm not going to approve patches that randomly > skip fields because it seems to get the right result sometimes. If > you want to try to explain what all this is actually trying to do, i'm > happy to work with you to come up with a sane solution. > Agreed. The Ada FE should either change or prove that there is a design/implementation flaw in the aliasing machinery.
> Agreed. The Ada FE should either change or prove that there is a > design/implementation flaw in the aliasing machinery. I think we can prove that there is a discrepancy between the Tree type-based aliasing analysis and the RTL type-based aliasing analysis when these flags are set, i.e. they don't agree on the alias set used to access some objects. Where they should meet is unclear for the time being...
Subject: Bug 25737 Author: nemet Date: Wed Jun 20 18:17:20 2007 New Revision: 125889 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=125889 Log: PR tree-optimization/25737 * testsuite/gcc.dg/tree-ssa/alias-14.c: New test. Added: trunk/gcc/testsuite/gcc.dg/tree-ssa/alias-14.c Modified: trunk/gcc/testsuite/ChangeLog
Subject: Bug 25737 Author: nemet Date: Wed Jun 20 18:19:10 2007 New Revision: 125890 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=125890 Log: PR tree-optimization/25737 * tree.h (struct tree_struct_field_tag): Add new field alias_set. (SFT_NONADDRESSABLE_P, SFT_ALIAS_SET): New macros. * tree-flow.h (struct fieldoff): Add new field alias_set. * tree-ssa-structalias.c (push_fields_onto_fieldstack): Add new argument addressable_type. Set alias_set of fieldoff. * tree-ssa-alias.c (create_sft): Add new argument alias_set. (create_overlap_variables_for): Pass alias_set from fieldoff to create_sft. * alias.c (get_alias_set): Use alias_set from SFT if set. Modified: trunk/gcc/ChangeLog trunk/gcc/alias.c trunk/gcc/tree-flow.h trunk/gcc/tree-ssa-alias.c trunk/gcc/tree-ssa-structalias.c trunk/gcc/tree.h
Subject: Bug 25737 Author: ebotcazou Date: Thu Jun 21 10:11:19 2007 New Revision: 125915 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=125915 Log: PR tree-optimization/25737 * misc.c (gnat_post_options): Do not force flag_tree_salias to 0. Modified: trunk/gcc/ada/ChangeLog trunk/gcc/ada/misc.c
Works on mainline.