When the attached file is compiled with a mingw32 build of GCC 4.2.0 using the -O2 option (gcc -O2 -c timestamp2.i), lines 159 and 160 are never executed when running the program that uses this file. Since the hashitem function modifies the variable b which is passed to it by-address, leaving those statements out is definitely not a valid optimization. Compiling without -O2 (gcc -c timestamp2.i) gives the correct result. Sorry I can't read assembler, or I'd try to narrow things down more. This is a regression from 4.1.2, which works correctly with or without -O2. If it helps, the GCC binaries I use (and sources) can be downloaded at http://hosted.filefront.com/tldragon7.
Created attachment 13701 [details] Preprocessed example source
Do you mean the last two stores to b: b->time = time; b->progress = found ? 4 : 2; Though we could have an alias violation if you don't cast back in hashitem to the correct type of the argument. This is still a bug.
(In reply to comment #2) > Do you mean the last two stores to b: > b->time = time; > b->progress = found ? 4 : 2; Yes, those two lines are the ones that are wrongly skipped.
(In reply to comment #2) > Though we could have an alias violation if you don't cast back in hashitem to > the correct type of the argument. hashitem() uses the type as punned to HASHDATA* (where HASHDATA is a struct with a single member of type char*). However, by implementation hashitem() will never assign a new address that doesn't point to the same type as what was passed -- in this case, it will only ever assign another BINDING*.
Is it possible that an optimization enabled by -O2 *assumes* that hashitem() will conform with strict aliasing by not dereferencing that argument, and thus optimizes those lines away? (Not the case.) If this is what is happening and is the correct behavior, then this is in fact user error and I'm sorry to have wasted your time.
Created attachment 13743 [details] Better testcase; compile with -O2, use with alias_main.c Compiling this file with -O2 and linking with the object file from alias_main.c creates a program that demonstrates the miscompilation. Curiously, compiling with -O2 and -fno-strict-aliasing produces a correct compilation.
Created attachment 13744 [details] Better testcase pt.2; use with alias1.c Compile this file with any or no additional options as desired; linking with the object file from alias1.c to create a program that demonstrates the miscompilation, if alias1.c was compiled with -O2.
Not seeing any further action or confirmation on this yet, I've gone ahead and created a simpler testcase. It's plain that, when using -O2, line 14 of alias1.c is skipped.
Confirmed. struct barstruct { char const* some_string; }; void changethepointer(struct barstruct**); void baz() { struct barstruct bar1; struct barstruct* barptr = &bar1; changethepointer(&barptr); barptr->some_string = "Everything's OK!"; } We end up with baz () { struct barstruct * barptr; struct barstruct bar1; struct barstruct * barptr.0; <bb 2>: # barptr_2 = V_MUST_DEF <barptr_1>; barptr = &bar1; # barptr_6 = V_MAY_DEF <barptr_2>; # SFT.1_7 = V_MAY_DEF <SFT.1_4>; # NONLOCAL.7_8 = V_MAY_DEF <NONLOCAL.7_5>; changethepointer (&barptr); # VUSE <barptr_6>; barptr.0_3 = barptr; # SFT.1_9 = V_MAY_DEF <SFT.1_7>; barptr.0_3->some_string = &"Everything\'s OK!"[0]; return; } because of a wrong points-to set again: barptr.0_3 = { bar1 } The constraints are Constraints: ANYTHING = &ANYTHING READONLY = &ANYTHING INTEGER = &ANYTHING ESCAPED_VARS = *ESCAPED_VARS NONLOCAL.7 = ESCAPED_VARS ESCAPED_VARS = &NONLOCAL.7 ESCAPED_VARS = &NONLOCAL.7 barptr = &bar1 ESCAPED_VARS = &bar1 ESCAPED_VARS = &barptr barptr.0_3 = barptr where ESCAPED_VARS = &barptr looks wrong? I'd say it needs to be ESCAPED_VARS = barptr instead. Danny?
trunk has the same problem, but different constraints: Constraints: ANYTHING = &ANYTHING READONLY = &ANYTHING INTEGER = &ANYTHING barptr = &bar1 barptr.0_1 = barptr Points-to sets NULL = { } ANYTHING = { ANYTHING } READONLY = { ANYTHING } INTEGER = { ANYTHING } barptr = { bar1 } bar1 = { } barptr.0_1 = same as barptr
Not sure if this is relevant or just GCC working differently on my target system, but I don't encounter the bug using -fstrict-aliasing, or in fact using individually the entire set of options under -O and -O2 on http://gcc.gnu.org/onlinedocs/gcc-4.2.0/gcc/Optimize-Options.html -- only when I specifically use the option "-O2". i.e. "gcc -fstrict-aliasing -c alias1.c" DOESN'T, for me, cause the bug but "gcc -O2 -c alias1.c" DOES.
Subject: Re: [4.2 Regression] -fstrict-aliasing causes skipped code On 20 Jun 2007 15:12:53 -0000, rguenth at gcc dot gnu dot org <gcc-bugzilla@gcc.gnu.org> wrote: > > > ------- Comment #9 from rguenth at gcc dot gnu dot org 2007-06-20 15:12 ------- > Confirmed. > > struct barstruct { char const* some_string; }; > > void changethepointer(struct barstruct**); > > void baz() > { > struct barstruct bar1; > struct barstruct* barptr = &bar1; > changethepointer(&barptr); > barptr->some_string = "Everything's OK!"; > } > > > We end up with > > baz () > { > struct barstruct * barptr; > struct barstruct bar1; > struct barstruct * barptr.0; > > <bb 2>: > # barptr_2 = V_MUST_DEF <barptr_1>; > barptr = &bar1; > # barptr_6 = V_MAY_DEF <barptr_2>; > # SFT.1_7 = V_MAY_DEF <SFT.1_4>; > # NONLOCAL.7_8 = V_MAY_DEF <NONLOCAL.7_5>; > changethepointer (&barptr); > # VUSE <barptr_6>; > barptr.0_3 = barptr; > # SFT.1_9 = V_MAY_DEF <SFT.1_7>; > barptr.0_3->some_string = &"Everything\'s OK!"[0]; > return; > > } > > because of a wrong points-to set again: Why do you believe this is wrong? It looks exactly right > > barptr.0_3 = { bar1 } > > The constraints are > > Constraints: > > ANYTHING = &ANYTHING > READONLY = &ANYTHING > INTEGER = &ANYTHING > ESCAPED_VARS = *ESCAPED_VARS > NONLOCAL.7 = ESCAPED_VARS > ESCAPED_VARS = &NONLOCAL.7 > ESCAPED_VARS = &NONLOCAL.7 > barptr = &bar1 > ESCAPED_VARS = &bar1 > ESCAPED_VARS = &barptr > barptr.0_3 = barptr > > where ESCAPED_VARS = &barptr looks wrong? I'd say it needs to be > ESCAPED_VARS = barptr instead. No, it's right on that part. It says ESCAPED_VARS points to barptr. ESCAPED_VARS = *ESCAPED_VARS will take care of making it point what barptr points to. What is missing here is that we should be doing barptr = ESCAPED_VARS when we see &barptr passed to the call.
(In reply to comment #10) > trunk has the same problem, but different constraints: > > Constraints: > > ANYTHING = &ANYTHING > READONLY = &ANYTHING > INTEGER = &ANYTHING > barptr = &bar1 > barptr.0_1 = barptr > > Points-to sets > > NULL = { } > ANYTHING = { ANYTHING } > READONLY = { ANYTHING } > INTEGER = { ANYTHING } > barptr = { bar1 } > bar1 = { } > barptr.0_1 = same as barptr > Same thing, when we see call(&barptr), we should say barptr = ANYTHING. .
Subject: Re: [4.2/4.3 Regression] -fstrict-aliasing causes skipped code On 4 Jul 2007 03:29:25 -0000, mmitchel at gcc dot gnu dot org <gcc-bugzilla@gcc.gnu.org> wrote: > > > -- Just as an update: I have been working with richi (I code, he tests :P) diligently on a patch for mainline, and have one that fixes the dealii regression (and thus, should fix this as well).
ping: anything that can be done for 4.2.1? This is a really serious regression.
Subject: Re: [4.2/4.3 Regression] -fstrict-aliasing causes skipped code I've attached a patch you should be able to quickly backport to 4.2.1. I'm still testing it against mainline right now. On 16 Jul 2007 13:39:20 -0000, giovannibajo at libero dot it <gcc-bugzilla@gcc.gnu.org> wrote: > > > ------- Comment #15 from giovannibajo at libero dot it 2007-07-16 13:39 ------- > ping: anything that can be done for 4.2.1? This is a really serious regression. >
Created attachment 13923 [details] 32328.diff
Created attachment 13924 [details] patch for 4.2 I'll bootstrap/regtest it.
The backported patch causes FAIL: gcc.c-torture/execute/pr28778.c execution, -O2 FAIL: gcc.c-torture/execute/pr28778.c execution, -Os DCE removes - -<L1>:; - list[3] = 42; we have one added constraint: blist.0_7 = blist_1 +*blist.0_7 = &ANYTHING ESCAPED_VARS = blist.0_7 IL differences after alias1 are @@ -251,12 +247,11 @@ list[3] = 42; blist_6 = &list; - # list_8 = PHI <list_4(3), list_5(4)>; # blist_1 = PHI <blist_3(3), blist_6(4)>; <L2>:; blist.0_7 = (const GLint *) blist_1; - # list_10 = V_MAY_DEF <list_8>; - # NONLOCAL.44_11 = V_MAY_DEF <NONLOCAL.44_9>; + # NONLOCAL.44_10 = V_MAY_DEF <NONLOCAL.44_8>; + # SMT.45_11 = V_MAY_DEF <SMT.45_9>; aglChoosePixelFormat (blist.0_7); return; we also have +NULL = &ANYTHING but this doesn't result in IL differences.
Subject: Re: [4.2/4.3 Regression] -fstrict-aliasing causes skipped code Oh, for 4.2 you need to add make_constraint_to_escaped_var On 16 Jul 2007 15:51:44 -0000, rguenth at gcc dot gnu dot org <gcc-bugzilla@gcc.gnu.org> wrote: > > > ------- Comment #19 from rguenth at gcc dot gnu dot org 2007-07-16 15:51 ------- > The backported patch causes > > FAIL: gcc.c-torture/execute/pr28778.c execution, -O2 > FAIL: gcc.c-torture/execute/pr28778.c execution, -Os > > DCE removes > > - > -<L1>:; > - list[3] = 42; > > we have one added constraint: > > blist.0_7 = blist_1 > +*blist.0_7 = &ANYTHING > ESCAPED_VARS = blist.0_7 > > IL differences after alias1 are > > @@ -251,12 +247,11 @@ > list[3] = 42; > blist_6 = &list; > > - # list_8 = PHI <list_4(3), list_5(4)>; > # blist_1 = PHI <blist_3(3), blist_6(4)>; > <L2>:; > blist.0_7 = (const GLint *) blist_1; > - # list_10 = V_MAY_DEF <list_8>; > - # NONLOCAL.44_11 = V_MAY_DEF <NONLOCAL.44_9>; > + # NONLOCAL.44_10 = V_MAY_DEF <NONLOCAL.44_8>; > + # SMT.45_11 = V_MAY_DEF <SMT.45_9>; > aglChoosePixelFormat (blist.0_7); > return; This is something that was fixed on 4.3 by proper SMT pruning. What is happening is it decides to prune the list access even though it's illegal to do so.
Subject: Bug 32328 Author: dberlin Date: Sun Aug 19 23:23:29 2007 New Revision: 127629 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=127629 Log: 2007-08-19 Daniel Berlin <dberlin@dberlin.org> Fix PR 32772 Fix PR 32716 Fix PR 32328 Fix PR 32303 * tree-flow.h (struct stmt_ann_d): Remove makes_clobbering_call. * tree-ssa-alias.c (init_transitive_clobber_worklist): Add on_worklist argument and avoid adding things to worklist multiple times. (add_to_worklist): Ditto. (mark_aliases_call_clobbered): Mark entire structure clobbered if single SFT is clobbered. (set_initial_properties): Ditto. (compute_call_clobbered): Update for changes to function arguments. (create_overlap_variables_for): Always create SFT for offset 0. (create_structure_vars): Handle PHI's, since we are in SSA form at this point. * tree-ssa-loop-ivopts.c (get_ref_tag): Don't return subvars. * tree-ssa-operands.c (access_can_touch_variable): Don't handle TARGET_MEM_REF. (add_vars_for_offset): Figure out aliases from access + points-to. (add_virtual_operand): Use add_vars-for_offset. (get_tmr_operands): Update for NMT changes, rewrite to be correct. (add_call_clobber_ops): Remove makes_clobbering_call set. (get_expr_operands): Always pass through the INDIRECT_REF reference. * tree-ssa-structalias.c (struct constraint_graph): Remove variables member. Add pe, pe_rep, pointer_label, loc_label, pointed_by, points_to, address_taken, pt_used, number_incoming. (FIRST_ADDR_NODE): Removed. (merge_graph_nodes): Remove broken code for the moment. (init_graph): New function. (build_pred_graph): Remove code to init_graph. Add location equivalence support. (struct scc_info): Rename roots to deleted. (scc_visit): Ditto. (init_scc_info): Ditto (init_topo_info): Use graph->size. (compute_topo_order): Ditto. (do_da_constraint): Removed. (do_sd_constraint): Remove calls to find(). set_union_with_increment should always get 0 as last arg here. (do_complex_constraint): Replace do_da_constraint with assert. Stop calling find. (struct equiv_class_label): New. (pointer_equiv_class_table): Ditto. (location_equiv_class_table): Ditto. (equiv_class_label_hash): Ditto. (equiv_class_label_eq): Ditto (equiv_class_lookup): Ditto. (equiv_class_ladd): Ditto. (pointer_equiv_class): Ditto. (location_equiv_class): Ditto. (condense_visit): Rename and rewrite from label_visit to do only SCC related stuff for HU. (label_visit): Do HU work for HU. (perform_var_substitution): Update to do HU and location equivalence. (free_var_substitution_info): Update to free HU and location equivalence structures. */ (find_equivalent_node): Update for pointer but not location equivalence. (unite_pointer_equivalences): New function. (move_complex_constraints): Rewrite to only do moving. (rewrite_constraints): Split out of move_complex_constraints. (solve_graph): Use graph->size. (process_constraint_1): Add from_call argument, use it. Split *a = &b into two constraints. (process_constraint): Use new process_constraint_1. (get_constraint_for_component_ref): Handle bitmaxsize == -1 case. (get_constraint_for): Handle non-pointer integers properly. Remove code that used to handle structures. (handle_ptr_arith): Fix a few bugs in pointer arithmetic handling with unknown addends. (handle_rhs_call): New function. (find_func_aliases): Use handle_rhs_call. (set_uids_in_ptset): Add an assert. (set_used_smts): Fix bug in not considering unified vars. (compute_tbaa_pruning): Stop initing useless iteration_obstack. (compute_points_to_sets): Update for other function changes. (delete_points_to_sets): Ditto. (ipa_pta_execute): Ditto. (pass_ipa_pta): We need to update SSA after ipa_pta. Modified: trunk/gcc/ChangeLog trunk/gcc/tree-flow.h trunk/gcc/tree-ssa-alias.c trunk/gcc/tree-ssa-loop-ivopts.c trunk/gcc/tree-ssa-operands.c trunk/gcc/tree-ssa-structalias.c
All should be fixed
But not with 4.2.
Subject: Bug 32328 Author: rguenth Date: Thu Aug 23 12:41:59 2007 New Revision: 127736 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=127736 Log: 2007-08-23 Richard Guenther <rguenther@suse.de> PR tree-optimization/32328 * testsuite/gcc.dg/pr32328.c: New testcase. Added: trunk/gcc/testsuite/gcc.dg/pr32328.c Modified: trunk/gcc/testsuite/ChangeLog
Daniel, can we backport this patch to 4.2, please? It's a P1 regression!
Not really.
Subject: Re: [4.2 Regression] -fstrict-aliasing causes skipped code On 5 Sep 2007 06:47:19 -0000, giovannibajo at libero dot it <gcc-bugzilla@gcc.gnu.org> wrote: > > > ------- Comment #25 from giovannibajo at libero dot it 2007-09-05 06:47 ------- > Daniel, can we backport this patch to 4.2, please? It's a P1 regression! > Not really. We've tried before, and it causes other regressions in 4.2 due to missing infrastructure.
Change target milestone to 4.2.3, as 4.2.2 has been released.
4.2.3 is being released now, changing milestones of open bugs to 4.2.4.
Are there sufficient motivation and resources to fix this bug in 4.2? Since 4.3 is soon-to-be, an update on this would be sincerely appreciated -- even if it's just forgetting about 4.2 and resolving as FIXED.
4.2.4 is being released, changing milestones to 4.2.5.
Closing 4.2 branch, fixed in 4.3.