Bug 31651 - [4.3 Regression] FRE does not fold intermediate CCP results, FRE does no longer look through loads
Summary: [4.3 Regression] FRE does not fold intermediate CCP results, FRE does no long...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.3.0
: P2 normal
Target Milestone: 4.3.0
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization
Depends on:
Blocks:
 
Reported: 2007-04-21 18:51 UTC by Richard Biener
Modified: 2007-06-30 14:17 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.2.0
Known to fail:
Last reconfirmed: 2007-04-28 00:48:15


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Biener 2007-04-21 18:51:13 UTC
Split out from PR31136.  Testcase:

struct S {
  unsigned b4:4;
  unsigned b6:6;
} s;

int main(void){
  s.b6 = 31;
  s.b4 = s.b6;
  s.b6 = s.b4;
  return s.b6 == 15 ? 0 : 1;
}

with gcc 4.2 FRE can look through s.b6 to see the 31 for the store to s.b4.
With mainline this no longer happens:

On 4.2 we have for the def_stmt

#   SFT.0D.1539_2 = V_MUST_DEF <SFT.0D.1539_1>;
sD.1526.b6D.1525 = 31

while on the trunk

# SFT.0_10 = VDEF <SFT.0_9(D)> { SFT.0 }
s.b6 = 31

and the predicate !ZERO_SSA_OPERANDS (def_stmt, SSA_OP_VIRTUAL_USES) evaluates
differently on them.  *sigh*


Also, create_value_expr_from does not fold the result even if it produces
constant arguments (generated by above look-through-loads).  This may
confuse fold (as it did in PR31136).
Comment 1 Richard Biener 2007-04-21 18:52:18 UTC
The missed look-through-loads is a regression from 4.2.  On the branch we also
do not fold constant intermediate results.
Comment 2 Andrew Pinski 2007-04-21 18:54:46 UTC
> and the predicate !ZERO_SSA_OPERANDS (def_stmt, SSA_OP_VIRTUAL_USES) evaluates differently on them.  *sigh*


This sounds more like this was introduced by the mem-ssa work, Diego?
Comment 3 Andrew Macleod 2007-04-21 22:23:17 UTC
I think this is actually fallout from the removal of V_MUSTDEFs, which predated mem-ssa going in.  Although it may have been checked into mainline at the same time as mem-ssa now that I think about it. The work was originally separate.

My guess would be that FRE was not updated at that time to deal properly with the absence of the V_MUSTDEF.  IIRC, MUSTDEFS were considered to NOT have any virtual operand USES, so this might now require a tuning of the ZERO_SSA_OPERANDS conditional to check the RHS if there is one and only one virtual def on the stmt to see whether its a relevant stmt or not.  

Since this is a missed optimization instead of a segfault or wrong code, it was probably easy to not notice it.
Comment 4 Andrew Pinski 2007-04-28 00:48:15 UTC
Confirmed, I noticed this also.
Comment 5 Daniel Berlin 2007-06-30 14:15:43 UTC
Subject: Bug 31651

Author: dberlin
Date: Sat Jun 30 14:15:26 2007
New Revision: 126149

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=126149
Log:
2007-06-30  Daniel Berlin  <dberlin@dberlin.org>
	
	Fix PR tree-optimization/32540
	Fix PR tree-optimization/31651

	* tree-ssa-sccvn.c: New file.

	* tree-ssa-sccvn.h: Ditto.
	
	* tree-vn.c: Include tree-ssa-sccvn.h
	(val_expr_paid_d): Removed.
	(value_table): Ditto.
	(vn_compute): Ditto.
	(val_expr_pair_hash): Ditto.
	(val_expr_pair_expr_eq): Ditto.
	(copy_vuses_from_stmt): Ditto.
	(vn_delete): Ditto.
	(vn_init): Ditto.
	(shared_vuses_from_stmt): Ditto.
	(print_creation_to_file): Moved up.
	(sort_vuses): Ditto.
	(sort_vuses_heap): Ditto.
	(set_value_handle): Make non-static.
	(make_value_handle): Ditto.
	(vn_add): Rewritten to use sccvn lookups.
	(vn_add_with_vuses): Ditto.
	(vn_lookup): Ditto (and second argument removed).
	(vn_lookup_with_vuses): Ditto.
	(vn_lookup_or_add): Ditto (and second argument removed);
	(vn_lookup_or_add_with_vuses): Ditto.
	(vn_lookup_with_stmt): New.
	(vn_lookup_or_add_with_stmt): Ditto.
	(create_value_handle_for_expr): Ditto.

	* tree-ssa-pre.c: Include tree-ssa-sccvn.h.
	(seen_during_translate): New function.
	(phi_trans_lookup): Use iterative_hash_expr, not vn_compute.
	(phi_trans_add): Ditto.
	(constant_expr_p): FIELD_DECL is always constant.
	(phi_translate_1): Renamed from phi_translate, add seen bitmap.
	Use constant_expr_p.
	Avoid infinite recursion on mutually valued expressions.
	Change callers of vn_lookup_or_add.
	(phi_translate): New function.
	(compute_antic_safe): Allow phi nodes.
	(create_component_ref_by_pieces): Update for FIELD_DECL change.
	(find_or_generate_expression): Rewrite slightly.
	(create_expression_by_pieces): Updated for vn_lookup_or_add
	change.
	Update VN_INFO for new names.
	(insert_into_preds_of_block): Update for new names.
	(add_to_exp_gen): New function.
	(add_to_sets): Use vn_lookup_or_add_with_stmt.
	(find_existing_value_expr): Rewrite to changed vn_lookup.
	(create_value_expr_from): Ditto, and use add_to_exp_gen.
	(try_look_through_load): Removed.
	(try_combine_conversion): Ditto.
	(get_sccvn_value): New function.
	(make_values_for_phi): Ditto.
	(make_values_for_stmt): Ditto.
	(compute_avail): Rewritten for vn_lookup_or_add changes and to use
	SCCVN.
	(init_pre): Update for SCCVN changes.
	(fini_pre): Ditto.
	(execute_pre): Ditto.

	* tree-flow.h (make_value_handle): Declare.
	(set_value_handle): Ditto.
	(sort_vuses_heap): Ditto.
	(vn_lookup_or_add_with_stmt): Ditto.
	(vn_lookup_with_stmt): Ditto.
	(vn_compute): Remove.
	(vn_init): Ditto.
	(vn_delete): Ditto.
	(vn_lookup): Update arguments.

	* Makefile.in (tree-ssa-pre.o): Add tree-ssa-sccvn.h
	(tree-vn.o): Ditto.
	(tree-ssa-sccvn.o): New.
	(OBJS-common): Add tree-ssa-sccvn.o
	


Added:
    trunk/gcc/testsuite/gcc.dg/tree-ssa/ssa-sccvn-1.c
      - copied unchanged from r125553, branches/gcc-pre-vn/gcc/testsuite/gcc.dg/tree-ssa/ssa-sccvn-1.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/ssa-sccvn-2.c
      - copied unchanged from r125553, branches/gcc-pre-vn/gcc/testsuite/gcc.dg/tree-ssa/ssa-sccvn-2.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/ssa-sccvn-3.c
      - copied unchanged from r125553, branches/gcc-pre-vn/gcc/testsuite/gcc.dg/tree-ssa/ssa-sccvn-3.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/ssa-sccvn-4.c
      - copied unchanged from r125553, branches/gcc-pre-vn/gcc/testsuite/gcc.dg/tree-ssa/ssa-sccvn-4.c
    trunk/gcc/tree-ssa-sccvn.c
      - copied, changed from r125553, branches/gcc-pre-vn/gcc/tree-ssa-sccvn.c
    trunk/gcc/tree-ssa-sccvn.h
      - copied, changed from r125553, branches/gcc-pre-vn/gcc/tree-ssa-sccvn.h
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/Makefile.in
    trunk/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-1.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-3.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-4.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-5.c
    trunk/gcc/tree-flow.h
    trunk/gcc/tree-ssa-operands.h
    trunk/gcc/tree-ssa-pre.c
    trunk/gcc/tree-vn.c

Comment 6 Daniel Berlin 2007-06-30 14:17:14 UTC
Fixed