Skip to content

Commit 602c6cf

Browse files
author
Jan Hubicka
committed
Improve handling of memory operands in ipa-icf 2/4
this patch iplements new class ao_compare that is derived from operand_compare and adds a method to compare and hash ao_refs. This is used by ICF to enable more merging. Comparsion is done as follows 1) Verify that the memory access will happen at the same address and will have same size. For constant addresses this is done by comparing ao_ref_base and offset/size For varable accesses it uses operand_equal_p but with OEP_ADDRESS (that does not match TBAA metadata) and then operand_equal_p on type size. 2) Compare alignments. I use get_object_alignment_1 like ipa-icf did before revamp to operand_equal_p in gcc 9. I noticed that return value is bitodd so added a comment 3) Match MR_DEPENDENCE_CLIQUE At this point the memory refrences are same except for TBAA information. We continue by checking 4) ref and base alias sets. Now if lto streaming is going to happen instead of comparing alias sets themselves we compare alias_ptr_types (the patch depends on the ao_ref_alias_ptr_tyep and ao_ref_base_alias_ptr_type acessors I sent yesterday) 5) See if accesses are view converted. If they are we are done since access path is not present 6) Compare the part of access path relevant for TBAA. I recall FRE relies on the fact that if base and ref types are same the access path is, but I do not thing this is 100% reliable especially with LTO alias sets. The access path comparsion logic is also useful for modref (for next stage1). Tracking the access paths improves quite noticeably disambiguation in C++ code by being able to distinquish different fields of same type within a struct. I had the comparsion logic in my tree for some time and it seems to work quite well. During cc1plus build we have some cases where we find mismatch after matching the base/ref alias sets. These are due to failed type merging: access path oracle in LTO uses TYPE_MAIN_VARIANTs. I implemented relatively basic hashing using base and offset. gcc/ChangeLog: * ipa-icf-gimple.c: Include tree-ssa-alias-compare.h. (find_checker::func_checker): Initialize m_tbaa. (func_checker::hash_operand): Use hash_ao_ref for memory accesses. (func_checker::compare_operand): Use compare_ao_refs for memory accesses. (func_checker::cmopare_gimple_assign): Do not check LHS types of memory stores. * ipa-icf-gimple.h (func_checker): Derive from ao_compare; add m_tbaa. * ipa-icf.c: Include tree-ssa-alias-compare.h. (sem_function::equals_private): Update call of func_checker::func_checker. * ipa-utils.h (lto_streaming_expected_p): New inline predicate. * tree-ssa-alias-compare.h: New file. * tree-ssa-alias.c: Include tree-ssa-alias-compare.h and bultins.h (view_converted_memref_p): New function. (types_equal_for_same_type_for_tbaa_p): New function. (ao_ref_alias_ptr_type, ao_ref_base_alias_ptr_type): New functions. (ao_compare::compare_ao_refs): New member function. (ao_compare::hash_ao_ref): New function * tree-ssa-alias.h (ao_ref_base_alias_ptr_type, ao_ref_alias_ptr_type): Declare. gcc/testsuite/ChangeLog: * c-c++-common/Wstringop-overflow-2.c: Disable ICF. * g++.dg/warn/Warray-bounds-8.C: Disable ICF.
1 parent a1fdc16 commit 602c6cf

File tree

9 files changed

+499
-24
lines changed

9 files changed

+499
-24
lines changed

gcc/ipa-icf-gimple.c

Lines changed: 56 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ along with GCC; see the file COPYING3. If not see
4040
#include "attribs.h"
4141
#include "gimple-walk.h"
4242

43+
#include "tree-ssa-alias-compare.h"
4344
#include "ipa-icf-gimple.h"
4445

4546
namespace ipa_icf_gimple {
@@ -52,13 +53,13 @@ namespace ipa_icf_gimple {
5253
of declarations that can be skipped. */
5354

5455
func_checker::func_checker (tree source_func_decl, tree target_func_decl,
55-
bool ignore_labels,
56+
bool ignore_labels, bool tbaa,
5657
hash_set<symtab_node *> *ignored_source_nodes,
5758
hash_set<symtab_node *> *ignored_target_nodes)
5859
: m_source_func_decl (source_func_decl), m_target_func_decl (target_func_decl),
5960
m_ignored_source_nodes (ignored_source_nodes),
6061
m_ignored_target_nodes (ignored_target_nodes),
61-
m_ignore_labels (ignore_labels)
62+
m_ignore_labels (ignore_labels), m_tbaa (tbaa)
6263
{
6364
function *source_func = DECL_STRUCT_FUNCTION (source_func_decl);
6465
function *target_func = DECL_STRUCT_FUNCTION (target_func_decl);
@@ -252,9 +253,16 @@ func_checker::hash_operand (const_tree arg, inchash::hash &hstate,
252253

253254
void
254255
func_checker::hash_operand (const_tree arg, inchash::hash &hstate,
255-
unsigned int flags, operand_access_type)
256+
unsigned int flags, operand_access_type access)
256257
{
257-
return hash_operand (arg, hstate, flags);
258+
if (access == OP_MEMORY)
259+
{
260+
ao_ref ref;
261+
ao_ref_init (&ref, const_cast <tree> (arg));
262+
return hash_ao_ref (&ref, lto_streaming_expected_p (), m_tbaa, hstate);
263+
}
264+
else
265+
return hash_operand (arg, hstate, flags);
258266
}
259267

260268
bool
@@ -314,18 +322,40 @@ func_checker::compare_operand (tree t1, tree t2, operand_access_type access)
314322
return true;
315323
else if (!t1 || !t2)
316324
return false;
317-
if (operand_equal_p (t1, t2, OEP_MATCH_SIDE_EFFECTS))
318-
return true;
319-
switch (access)
325+
if (access == OP_MEMORY)
320326
{
321-
case OP_MEMORY:
322-
return return_false_with_msg
323-
("operand_equal_p failed (access == memory)");
324-
case OP_NORMAL:
327+
ao_ref ref1, ref2;
328+
ao_ref_init (&ref1, const_cast <tree> (t1));
329+
ao_ref_init (&ref2, const_cast <tree> (t2));
330+
int flags = compare_ao_refs (&ref1, &ref2,
331+
lto_streaming_expected_p (), m_tbaa);
332+
333+
if (!flags)
334+
return true;
335+
if (flags & SEMANTICS)
336+
return return_false_with_msg
337+
("compare_ao_refs failed (semantic difference)");
338+
if (flags & BASE_ALIAS_SET)
339+
return return_false_with_msg
340+
("compare_ao_refs failed (base alias set difference)");
341+
if (flags & REF_ALIAS_SET)
342+
return return_false_with_msg
343+
("compare_ao_refs failed (ref alias set difference)");
344+
if (flags & ACCESS_PATH)
345+
return return_false_with_msg
346+
("compare_ao_refs failed (access path difference)");
347+
if (flags & DEPENDENCE_CLIQUE)
348+
return return_false_with_msg
349+
("compare_ao_refs failed (dependence clique difference)");
350+
gcc_unreachable ();
351+
}
352+
else
353+
{
354+
if (operand_equal_p (t1, t2, OEP_MATCH_SIDE_EFFECTS))
355+
return true;
325356
return return_false_with_msg
326-
("operand_equal_p failed (access == normal)");
357+
("operand_equal_p failed");
327358
}
328-
gcc_unreachable ();
329359
}
330360

331361
bool
@@ -593,10 +623,17 @@ func_checker::compare_gimple_call (gcall *s1, gcall *s2)
593623

594624
tree fntype1 = gimple_call_fntype (s1);
595625
tree fntype2 = gimple_call_fntype (s2);
596-
if ((fntype1 && !fntype2)
597-
|| (!fntype1 && fntype2)
598-
|| (fntype1 && !types_compatible_p (fntype1, fntype2)))
599-
return return_false_with_msg ("call function types are not compatible");
626+
627+
/* For direct calls we verify that types are comopatible so if we matced
628+
callees, callers must match, too. For indirect calls however verify
629+
function type. */
630+
if (!gimple_call_fndecl (s1))
631+
{
632+
if ((fntype1 && !fntype2)
633+
|| (!fntype1 && fntype2)
634+
|| (fntype1 && !types_compatible_p (fntype1, fntype2)))
635+
return return_false_with_msg ("call function types are not compatible");
636+
}
600637

601638
if (fntype1 && fntype2 && comp_type_attributes (fntype1, fntype2) != 1)
602639
return return_false_with_msg ("different fntype attributes");
@@ -652,10 +689,10 @@ func_checker::compare_gimple_assign (gimple *s1, gimple *s2)
652689
arg2 = gimple_op (s2, i);
653690

654691
/* Compare types for LHS. */
655-
if (i == 0)
692+
if (i == 0 && !gimple_store_p (s1))
656693
{
657694
if (!compatible_types_p (TREE_TYPE (arg1), TREE_TYPE (arg2)))
658-
return return_false_with_msg ("GIMPLE NOP LHS type mismatch");
695+
return return_false_with_msg ("GIMPLE LHS type mismatch");
659696
}
660697

661698
if (!compare_operand (arg1, arg2, get_operand_access_type (&map, arg1)))

gcc/ipa-icf-gimple.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,14 +118,14 @@ class sem_bb
118118

119119
/* A class aggregating all connections and semantic equivalents
120120
for a given pair of semantic function candidates. */
121-
class func_checker : operand_compare
121+
class func_checker : ao_compare
122122
{
123123
public:
124124
/* Default constructor. */
125125
func_checker ():
126126
m_source_func_decl (NULL_TREE), m_target_func_decl (NULL_TREE),
127127
m_ignored_source_nodes (NULL), m_ignored_target_nodes (NULL),
128-
m_ignore_labels (false)
128+
m_ignore_labels (false), m_tbaa (true)
129129
{
130130
m_source_ssa_names.create (0);
131131
m_target_ssa_names.create (0);
@@ -139,6 +139,7 @@ class func_checker : operand_compare
139139
of declarations that can be skipped. */
140140
func_checker (tree source_func_decl, tree target_func_decl,
141141
bool ignore_labels = false,
142+
bool tbaa = true,
142143
hash_set<symtab_node *> *ignored_source_nodes = NULL,
143144
hash_set<symtab_node *> *ignored_target_nodes = NULL);
144145

@@ -275,6 +276,9 @@ class func_checker : operand_compare
275276
/* Flag if ignore labels in comparison. */
276277
bool m_ignore_labels;
277278

279+
/* Flag if we should compare type based alias analysis info. */
280+
bool m_tbaa;
281+
278282
public:
279283
/* Return true if two operands are equal. The flags fields can be used
280284
to specify OEP flags described above. */

gcc/ipa-icf.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ along with GCC; see the file COPYING3. If not see
7878
#include "attribs.h"
7979
#include "print-tree.h"
8080
#include "ipa-utils.h"
81+
#include "tree-ssa-alias-compare.h"
8182
#include "ipa-icf-gimple.h"
8283
#include "fibonacci_heap.h"
8384
#include "ipa-icf.h"
@@ -843,6 +844,8 @@ sem_function::equals_private (sem_item *item)
843844

844845
m_checker = new func_checker (decl, m_compared_func->decl,
845846
false,
847+
opt_for_fn (m_compared_func->decl,
848+
flag_strict_aliasing),
846849
&refs_set,
847850
&m_compared_func->refs_set);
848851
arg1 = DECL_ARGUMENTS (decl);

gcc/ipa-utils.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,4 +265,16 @@ get_odr_name_for_type (tree type)
265265
return IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (type_name));
266266
}
267267

268+
/* Return true if we are going to do LTO streaming. */
269+
270+
inline bool
271+
lto_streaming_expected_p ()
272+
{
273+
/* Compilation before LTO stremaing. */
274+
if (flag_lto && !in_lto_p && symtab->state < IPA_SSA_AFTER_INLINING)
275+
return true;
276+
/* WPA or incremental link. */
277+
return (flag_wpa || flag_incremental_link == INCREMENTAL_LINK_LTO);
278+
}
279+
268280
#endif /* GCC_IPA_UTILS_H */

gcc/testsuite/c-c++-common/Wstringop-overflow-2.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/* PR middle-end/91458 - inconsistent warning for writing past the end
22
of an array member
33
{ dg-do compile }
4-
{ dg-options "-O2 -Wall -Wno-array-bounds" } */
4+
{ dg-options "-O2 -Wall -Wno-array-bounds -fno-ipa-icf" } */
55

66
void sink (void*);
77

gcc/testsuite/g++.dg/warn/Warray-bounds-8.C

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
See Wstringop-overflow-3.C for the same test that exercises the other
44
warning.
55
{ dg-do compile }
6-
{ dg-options "-O2 -Wall -Wno-stringop-overflow" }
6+
{ dg-options "-O2 -Wall -Wno-stringop-overflow -fno-ipa-icf" }
77
{ dg-skip-if "" { *-*-aix* } } */
88

99
void sink (void*);

gcc/tree-ssa-alias-compare.h

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/* Comparsion of AO ref.
2+
Copyright (C) 2020 Free Software Foundation, Inc.
3+
4+
This file is part of GCC.
5+
6+
GCC is free software; you can redistribute it and/or modify
7+
under the terms of the GNU General Public License as published by
8+
the Free Software Foundation; either version 3 of the License, or
9+
(at your option) any later version.
10+
11+
GCC is distributed in the hope that it will be useful,
12+
but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
14+
GNU General Public License for more details.
15+
16+
You should have received a copy of the GNU General Public License
17+
along with GCC; see the file COPYING3. If not see
18+
<http://www.gnu.org/licenses/>. */
19+
20+
#ifndef TREE_SSA_ALIAS_COMPARE_H
21+
#define TREE_SSA_ALIAS_COMPARE_H
22+
23+
class operand_compare;
24+
/* A class aggregating all connections and semantic equivalents
25+
for a given pair of semantic function candidates. */
26+
class ao_compare : public operand_compare
27+
{
28+
public:
29+
enum ao_ref_diff
30+
{
31+
SEMANTICS = 1,
32+
BASE_ALIAS_SET = 2,
33+
REF_ALIAS_SET = 4,
34+
ACCESS_PATH = 8,
35+
DEPENDENCE_CLIQUE = 16
36+
};
37+
int compare_ao_refs (ao_ref *ref1, ao_ref *ref2, bool lto_streaming_safe,
38+
bool tbaa);
39+
void hash_ao_ref (ao_ref *ref, bool lto_streaming_safe, bool tbaa,
40+
inchash::hash &hstate);
41+
};
42+
43+
#endif

0 commit comments

Comments
 (0)