This finally tries to solve the issue that PRE uses the VN tables and their expression representation for insertion. While for VN things like alignment and aliasing do not matter it is quite important to insert expressions that are compatible with the original one which means not blindly take the canonical expression from the VN tables.
This patch fixes the issue in the least invasive way by only making sure the canonical expression is conservative for all expressions visited during PRE avail computation. The issue hits via gcc.dg/torture/pr65270-1.c (formerly a tailmerging bug) and code hoisting but I'm sure that with enough luck one can construct a PRE testcase as well (it may need artificial disabling of earlier memory CSE passes though). Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. Richard. 2016-07-07 Richard Biener <rguent...@suse.de> * tree-ssa-pre.c: Include alias.h. (compute_avail): If we have multiple VN_REFERENCEs with the same hashtable entry adjust that to make it a valid replacement for all of them with respect to alignment and aliasing when doing insertion. * tree-ssa-sccvn.h (vn_reference_operands_for_lookup): Declare. * tree-ssa-sccvn.c (vn_reference_operands_for_lookup): New function. Index: gcc/tree-ssa-pre.c =================================================================== *** gcc/tree-ssa-pre.c (revision 238003) --- gcc/tree-ssa-pre.c (working copy) *************** along with GCC; see the file COPYING3. *** 53,58 **** --- 53,59 ---- #include "ipa-utils.h" #include "tree-cfgcleanup.h" #include "langhooks.h" + #include "alias.h" /* TODO: *************** compute_avail (void) *** 3724,3735 **** case VN_REFERENCE: { vn_reference_t ref; ! vn_reference_lookup (gimple_assign_rhs1 (stmt), ! gimple_vuse (stmt), ! VN_WALK, &ref, true); if (!ref) ! continue; /* If the value of the reference is not invalidated in this block until it is computed, add the expression --- 3725,3743 ---- case VN_REFERENCE: { + tree rhs1 = gimple_assign_rhs1 (stmt); + alias_set_type set = get_alias_set (rhs1); + vec<vn_reference_op_s> operands + = vn_reference_operands_for_lookup (rhs1); vn_reference_t ref; ! vn_reference_lookup_pieces (gimple_vuse (stmt), set, ! TREE_TYPE (rhs1), ! operands, &ref, VN_WALK); if (!ref) ! { ! operands.release (); ! continue; ! } /* If the value of the reference is not invalidated in this block until it is computed, add the expression *************** compute_avail (void) *** 3753,3759 **** = SSA_NAME_DEF_STMT (gimple_vuse (def_stmt)); } if (!ok) ! continue; } result = pre_expr_pool.allocate (); --- 3761,3828 ---- = SSA_NAME_DEF_STMT (gimple_vuse (def_stmt)); } if (!ok) ! { ! operands.release (); ! continue; ! } ! } ! ! /* If the load was value-numbered to another ! load make sure we do not use its expression ! for insertion if it wouldn't be a valid ! replacement. */ ! /* At the momemt we have a testcase ! for hoist insertion of aligned vs. misaligned ! variants in gcc.dg/torture/pr65270-1.c thus ! with just alignment to be considered we can ! simply replace the expression in the hashtable ! with the most conservative one. */ ! vn_reference_op_t ref1 = &ref->operands.last (); ! while (ref1->opcode != TARGET_MEM_REF ! && ref1->opcode != MEM_REF ! && ref1 != &ref->operands[0]) ! --ref1; ! vn_reference_op_t ref2 = &operands.last (); ! while (ref2->opcode != TARGET_MEM_REF ! && ref2->opcode != MEM_REF ! && ref2 != &operands[0]) ! --ref2; ! if ((ref1->opcode == TARGET_MEM_REF ! || ref1->opcode == MEM_REF) ! && (TYPE_ALIGN (ref1->type) ! > TYPE_ALIGN (ref2->type))) ! { ! ref->operands.release (); ! ref->operands = operands; ! ref1 = ref2; ! } ! else ! operands.release (); ! /* TBAA behavior is an obvious part so make sure ! that the hashtable one covers this as well ! by adjusting the ref alias set and its base. */ ! if (ref->set == set ! || alias_set_subset_of (set, ref->set)) ! ; ! else if (alias_set_subset_of (ref->set, set)) ! { ! ref->set = set; ! if (ref1->opcode == MEM_REF) ! ref1->op0 = fold_convert (TREE_TYPE (ref2->op0), ! ref1->op0); ! else ! ref1->op2 = fold_convert (TREE_TYPE (ref2->op2), ! ref1->op2); ! } ! else ! { ! ref->set = 0; ! if (ref1->opcode == MEM_REF) ! ref1->op0 = fold_convert (ptr_type_node, ! ref1->op0); ! else ! ref1->op2 = fold_convert (ptr_type_node, ! ref1->op2); } result = pre_expr_pool.allocate (); Index: gcc/tree-ssa-sccvn.c =================================================================== *** gcc/tree-ssa-sccvn.c (revision 238003) --- gcc/tree-ssa-sccvn.c (working copy) *************** vn_reference_lookup_3 (ao_ref *ref, tree *** 2285,2290 **** --- 2285,2301 ---- return (void *)-1; } + /* Return a reference op vector from OP that can be used for + vn_reference_lookup_pieces. The caller is responsible for releasing + the vector. */ + + vec<vn_reference_op_s> + vn_reference_operands_for_lookup (tree op) + { + bool valueized; + return valueize_shared_reference_ops_from_ref (op, &valueized).copy (); + } + /* Lookup a reference operation by it's parts, in the current hash table. Returns the resulting value number if it exists in the hash table, NULL_TREE otherwise. VNRESULT will be filled in with the actual Index: gcc/tree-ssa-sccvn.h =================================================================== *** gcc/tree-ssa-sccvn.h (revision 238003) --- gcc/tree-ssa-sccvn.h (working copy) *************** vn_nary_op_t vn_nary_op_insert_pieces (u *** 214,219 **** --- 214,220 ---- tree, tree *, tree, unsigned int); bool ao_ref_init_from_vn_reference (ao_ref *, alias_set_type, tree, vec<vn_reference_op_s> ); + vec<vn_reference_op_s> vn_reference_operands_for_lookup (tree); tree vn_reference_lookup_pieces (tree, alias_set_type, tree, vec<vn_reference_op_s> , vn_reference_t *, vn_lookup_kind);