Re: [patch, avr] Take 2: Fix PR64331: insn output and insn length computation rely on REG_DEAD notes.
2015-02-27 1:45 GMT+03:00 Steven Bosscher stevenb@gmail.com: On Thu, Feb 26, 2015 at 8:35 PM, Georg-Johann Lay wrote: Take #2 introduces a new, avr-specific rtl pass whose sole purpose is to rectify notes. The pass is scheduled right before cfg does down (right before .*free_cfg) so that cfg and hence df machinery is available. Regression tests look fine and for the test case the patches produce correct code and correct insn length. Sorry for party-pooping, but it seems to me that the real bug is that the target is lying to reload. IIUC the AVR port selects the insn alternative after register allocation (after reload). It bases its selection on REG_DEAD notes. In PR64331 an alternative is used that clobbers r20 that has a REG_DEAD note, but r20 is not actually dead because hardreg-cprop has propagated it forward without adjusting the note. The normal way of things is that the insn alternative is selected in reload (or in LRA) and that the clobbers are added as necessary. In PR64331, an alternative for insn r17 would be selected that has a CLOBBER for r20, prevent hardreg-cprop from propagating r20. Selecting insns based on REG-notes is dangerous business. Lying to reload and to post-RA passes is a mortal sin, the compiler will punish you. There is no guarantee that nothing will change between your new pass to recompute notes, and the final pass that emits the insns. It's not my port, for sure, but I would look for a real fix instead: Don't select insns to output based on unreliable information like REG-notes. Steven rights. We will have an endless fight with this problem. Better to completely drop `reg_unused_after'. (I know that it used around 40 times in port) What do you think Georg ? Denis.
Re: [PATCH][AArch64]: Fix rtl type in aarch64.md.
On Sat, Feb 28, 2015 at 01:29:15AM +, Xingxing Pan wrote: On 02/27/2015 04:30 PM, Marcus Shawcroft wrote: On 26 February 2015 at 06:22, Xingxing Pan xxing...@marvell.com wrote: This patch fix the type of movmode_aarch64 in aarch64.md. Is it OK for trunk? OK, thank you /Marcus Could someone help to apply the patch? Until now I don't have SVN write access. Thanks for the patch, I've committed it on your behalf as revision 221075. Cheers, James
Re: [PATCH][AArch64]: Fix rtl type in aarch64.md.
On 02/28/2015 04:39 PM, James Greenhalgh wrote: On Sat, Feb 28, 2015 at 01:29:15AM +, Xingxing Pan wrote: On 02/27/2015 04:30 PM, Marcus Shawcroft wrote: On 26 February 2015 at 06:22, Xingxing Pan xxing...@marvell.com wrote: This patch fix the type of movmode_aarch64 in aarch64.md. Is it OK for trunk? OK, thank you /Marcus Could someone help to apply the patch? Until now I don't have SVN write access. Thanks for the patch, I've committed it on your behalf as revision 221075. Cheers, James Thanks. -- Regards, Xingxing
Re: ipa-icf::merge TLC
On Fri, Feb 27, 2015 at 02:10:47AM +, Jan Hubicka wrote: * ipa-icf.c (symbol_compare_collection::symbol_compare_colleciton): Use address_matters_p. (redirect_all_callers, set_addressable): New functions. (sem_function::merge): Reorganize and fix merging issues. (sem_variable::merge): Likewise. (sem_variable::compare_sections): Remove. * common.opt (fmerge-all-constants, fmerge-constants): Remove Optimization flag. * symtab.c (symtab_node::resolve_alias): When alias has aliases, redirect them. (symtab_node::make_decl_local): Set ADDRESSABLE bit when decl is used. (address_matters_1): New function. (symtab_node::address_matters_p): New function. * cgraph.c (cgraph_edge::verify_corresponds_to_fndecl): Fix check for merged flag. * cgraph.h (address_matters_p): Declare. (symtab_node::address_taken_from_non_vtable_p): Remove. (symtab_node::address_can_be_compared_p): New method. (ipa_ref::address_matters_p): Move here from ipa-ref.c; simplify. * ipa-visibility.c (symtab_node::address_taken_from_non_vtable_p): Remove. (comdat_can_be_unshared_p_1) Use address_matters_p. (update_vtable_references): Fix formating. * ipa-ref.c (ipa_ref::address_matters_p): Move inline. * cgraphunit.c (cgraph_node::create_wrapper): Drop UNINLINABLE flag. * cgraphclones.c: Preserve merged and icf_merged flags. * gcc.dg/pr64454.c: Disable ICF. * gcc.dg/pr28685-1.c: Disable ICF * gcc.dg/ipa/iinline-5.c: Disable ICF. * g++.dg/warn/Wsuggest-final.C: Force methods to be different. * g++.dg/ipa/ipa-icf-4.C: Update template. Hi Honza, This is more a note for other interested AArch64 testers, but this patch breaks some tests on aarch64-none-elf. Looking at the test output, this is a problem with the tests than with your patch. We now eliminate some function bodies which are identical across test functions, causing us to fail some scan-assembler directives. The fix is straightforward, we just need to add -fno-ipa-icf to get things working again. This keeps the spirit of the original tests. I'll propose a patch along those lines on Monday. Thanks, James --- PASS-FAIL: gcc.target/aarch64/atomic-comp-swap-release-acquire.c scan-assembler-times ldaxr\tw[0-9]+, \\[x[0-9]+\\] 4 PASS-FAIL: gcc.target/aarch64/atomic-comp-swap-release-acquire.c scan-assembler-times stlxr\tw[0-9]+, w[0-9]+, \\[x[0-9]+\\] 4 PASS-FAIL: gcc.target/aarch64/vect_saddl_1.c scan-assembler-times saddl2\tv[0-9]+.2d, v[0-9]+.4s, v[0-9]+.4s 2 PASS-FAIL: gcc.target/aarch64/vect_saddl_1.c scan-assembler-times saddl\tv[0-9]+.2d, v[0-9]+.2s, v[0-9]+.2s 2 PASS-FAIL: gcc.target/aarch64/vect_saddl_1.c scan-assembler-times ssubl2\tv[0-9]+.2d, v[0-9]+.4s, v[0-9]+.4s 5 PASS-FAIL: gcc.target/aarch64/vect_saddl_1.c scan-assembler-times ssubl\tv[0-9]+.2d, v[0-9]+.2s, v[0-9]+.2s 5 PASS-FAIL: gcc.target/aarch64/vect_smlal_1.c scan-assembler-times smlsl2\tv[0-9]+.8h 5 PASS-FAIL: gcc.target/aarch64/vect_smlal_1.c scan-assembler-times smlsl\tv[0-9]+.8h 5
Re: [patch] Fix ICE on unaligned record field
for various reasons I was not able to do it earlier, but today I have re-bootstrapped the following (the only change is the added testcase) on x86_64-linux and it passes OK. Should I commit it to trunk then? Yes, that would be kind of you, thanks in advance. -- Eric Botcazou
Fix PR ipa/65232 (icf_merge breaking anchors)
Hi,
this patch fixes ICE where we produce RTL because of notice_global_symbol
but this RTL is no longer valid after turning symbol into an alias.
Bootstrapped/regtested x86_64-linux, will commit it shortly.
Honza
PR ipa/65232
* ipa-icf.c (clear_decl_rtl): New function.
(sem_function::merge): Clear RTL before forming alias.
(sem_variable::merge): Clear RTL before forming alias.
Index: ipa-icf.c
===
--- ipa-icf.c (revision 221076)
+++ ipa-icf.c (working copy)
@@ -632,7 +632,7 @@ set_local (cgraph_node *node, void *data
return false;
}
-/* TREE_ADDRESSABLE of NODE to true if DATA is non-NULL.
+/* TREE_ADDRESSABLE of NODE to true.
Helper for call_for_symbol_thunks_and_aliases. */
static bool
@@ -642,6 +642,16 @@ set_addressable (varpool_node *node, voi
return false;
}
+/* Clear DECL_RTL of NODE.
+ Helper for call_for_symbol_thunks_and_aliases. */
+
+static bool
+clear_decl_rtl (symtab_node *node, void *)
+{
+ SET_DECL_RTL (node-decl, NULL);
+ return false;
+}
+
/* Redirect all callers of N and its aliases to TO. Remove aliases if
possible. Return number of redirections made. */
@@ -893,6 +908,9 @@ sem_function::merge (sem_item *alias_ite
ipa_merge_profiles (original, alias);
alias-release_body (true);
alias-reset ();
+ /* Notice global symbol possibly produced RTL. */
+ ((symtab_node *)alias)-call_for_symbol_and_aliases (clear_decl_rtl,
+ NULL, true);
/* Create the alias. */
cgraph_node::create_alias (alias_func-decl, decl);
@@ -1512,6 +1543,8 @@ sem_variable::merge (sem_item *alias_ite
alias-analyzed = false;
DECL_INITIAL (alias-decl) = NULL;
+ ((symtab_node *)alias)-call_for_symbol_and_aliases (clear_decl_rtl,
+ NULL, true);
alias-need_bounds_init = false;
alias-remove_all_references ();
if (TREE_ADDRESSABLE (alias-decl))
Re: [PATCH] ICF is more strict about non-common function and var, attributes.
2015-02-28 Martin Liska mli...@suse.cz Jan Hubicka hubi...@ucw.cz * ipa-icf-gimple.c (func_checker::compare_variable_decl): Validate variable alignment. * ipa-icf.c (sem_function::equals_private): Be more precise about non-common function attributes. (sem_variable::equals): Likewise. OK, does this fix gcc.target/i386/stackalign/longlong-2.c -mno-stackrealign scan-assembler-times and[lq]?[^\\n]*-16,[^\\n]*sp 2 FAIL: gcc.target/i386/stackalign/longlong-2.c -mno-stackrealign scan-assembler-times and[lq]?[^\\n]*-8,[^\\n]*sp 2 FAIL: gcc.target/i386/stackalign/longlong-2.c -mstackrealign scan-assembler-times and[lq]?[^\\n]*-16,[^\\n]*sp 2 FAIL: gcc.target/i386/stackalign/longlong-2.c -mstackrealign scan-assembler-times and[lq]?[^\\n]*-8,[^\\n]*sp 2 on -m32 mentioned in PR65237? Honza
Re: [PATCH] Fix for PR ipa/65245
Hello.
Following patch skips all variable aliases as potential merge candidates.
Tested on x86_64-linux.
Ready for trunk?
Thanks,
Martin
From fbde2e98f98a71105d18cf3e91e8032d0c657139 Mon Sep 17 00:00:00 2001
From: mliska mli...@suse.cz
Date: Fri, 27 Feb 2015 22:42:49 +0100
Subject: [PATCH 2/4] ICF: Do not consider variable aliases for merge
operation.
gcc/ChangeLog:
2015-02-28 Martin Liska mli...@suse.cz
Jan Hubicka hubi...@ucw.cz
PR ipa/65245
* ipa-icf.c (sem_function::parse): Do not consider aliases.
(sem_variable::parse): Likewise.
(sem_item_optimizer::build_graph): Consider ultimate aliases
for references.
OK, can you please add a testcase checking that two functions get merged even
if each of them referrs different alias of an equvalent symbols?
Honza
---
gcc/ipa-icf.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index 92133fc..864a5d0 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -1095,7 +1095,7 @@ sem_function::parse (cgraph_node *node, bitmap_obstack
*stack)
tree fndecl = node-decl;
function *func = DECL_STRUCT_FUNCTION (fndecl);
- /* TODO: add support for thunks and aliases. */
+ /* TODO: add support for thunks. */
if (!func || !node-has_gimple_body_p ())
return NULL;
@@ -1407,6 +1407,9 @@ sem_variable::parse (varpool_node *node, bitmap_obstack
*stack)
{
tree decl = node-decl;
+ if (node-alias)
+return NULL;
+
bool readonly = TYPE_P (decl) ? TYPE_READONLY (decl) : TREE_READONLY
(decl);
if (!readonly)
return NULL;
@@ -2057,7 +2060,8 @@ sem_item_optimizer::build_graph (void)
cgraph_edge *e = cnode-callees;
while (e)
{
- sem_item **slot = m_symtab_node_map.get (e-callee);
+ sem_item **slot = m_symtab_node_map.get
+ (e-callee-ultimate_alias_target ());
if (slot)
item-add_reference (*slot);
@@ -2068,7 +2072,8 @@ sem_item_optimizer::build_graph (void)
ipa_ref *ref = NULL;
for (unsigned i = 0; item-node-iterate_reference (i, ref); i++)
{
- sem_item **slot = m_symtab_node_map.get (ref-referred);
+ sem_item **slot = m_symtab_node_map.get
+ (ref-referred-ultimate_alias_target ());
if (slot)
item-add_reference (*slot);
}
--
2.1.2
[PATCH] ICF: move readonly decision for variables to the right place
Hello.
This patch makes correctly decision about variables that are read-only and are
candidates
for merge operation. Tested on x86_64-unknown-linux-gnu.
Ready for trunk?
Thanks,
Martin
From 72370368eb04a69bf36b42f692d44c21592bc94a Mon Sep 17 00:00:00 2001
From: mliska mli...@suse.cz
Date: Fri, 27 Feb 2015 23:44:32 +0100
Subject: [PATCH 3/4] ICF: Optimize variable merging.
gcc/ChangeLog:
2015-02-28 Martin Liska mli...@suse.cz
Jan Hubicka hubi...@ucw.cz
* ipa-icf.c (sem_variable::parse): Do not filter variables
too early.
(sem_item_optimizer::filter_removed_items): Filter out correct
readonly variables.
gcc/testsuite/ChangeLog:
2015-02-28 Martin Liska mli...@suse.cz
Jan Hubicka hubi...@ucw.cz
* gcc.dg/ipa/ipa-icf-35.c: New test.
---
gcc/ipa-icf.c | 13 -
gcc/testsuite/gcc.dg/ipa/ipa-icf-35.c | 34 ++
2 files changed, 42 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/ipa/ipa-icf-35.c
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index 864a5d0..5b1fcff 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -1410,10 +1410,6 @@ sem_variable::parse (varpool_node *node, bitmap_obstack *stack)
if (node-alias)
return NULL;
- bool readonly = TYPE_P (decl) ? TYPE_READONLY (decl) : TREE_READONLY (decl);
- if (!readonly)
-return NULL;
-
bool can_handle = DECL_VIRTUAL_P (decl)
|| flag_merge_constants = 2
|| (!TREE_ADDRESSABLE (decl) !node-externally_visible);
@@ -1900,7 +1896,14 @@ sem_item_optimizer::filter_removed_items (void)
if (!flag_ipa_icf_variables)
remove_item (item);
else
- filtered.safe_push (item);
+ {
+ /* Filter out non-readonly variables. */
+ tree decl = item-decl;
+ if (TYPE_P (decl) ? TYPE_READONLY (decl) : TREE_READONLY (decl))
+ filtered.safe_push (item);
+ else
+ remove_item (item);
+ }
}
}
diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-icf-35.c b/gcc/testsuite/gcc.dg/ipa/ipa-icf-35.c
new file mode 100644
index 000..95d247e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-35.c
@@ -0,0 +1,34 @@
+/* { dg-do compile } */
+/* { dg-options -O2 -fdump-ipa-icf } */
+
+#include stdlib.h
+#include assert.h
+
+void f1()
+{
+}
+
+void f2()
+{
+}
+
+static void (*a)(void)=f1;
+static void (*b)(void)=f1;
+static void (*c)(void)=f2;
+static void (*d)(void)=f2;
+
+int main()
+{
+ a();
+ b();
+ c();
+ d();
+
+ return 0;
+}
+
+/* { dg-final { scan-ipa-dump Equal symbols: 3 icf } } */
+/* { dg-final { scan-ipa-dump Semantic equality hit:f2-f1 icf } } */
+/* { dg-final { scan-ipa-dump Semantic equality hit:d-c icf } } */
+/* { dg-final { scan-ipa-dump Semantic equality hit:b-a icf } } */
+/* { dg-final { cleanup-ipa-dump icf } } */
--
2.1.2
Re: Fix thunk generation for functions returning DECL_BY_REFERENCE
Jan Hubicka hubi...@ucw.cz writes:
+struct A {
+ A() {ptr=b;}
+ A(const A a) {ptr = b;}
+ void test() { if (ptr != b) __builtin_abort ();}
+ int b;
+ int *ptr;
+};
Please remove the trailing whitespace.
Andreas.
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
And now for something completely different.
Re: [PATCH] ICF is more strict about non-common function and var, attributes.
Half of FAILs are gone, but the rest is correctly merged (alignment matches). Thus I would omit ICF in this testcase. I'm going to install the patch. Richard, I wonder what happens with TYPE_ALIGN at LTO. It is not part of canonical type definition and thus we get random alignments on CANONICAL_TYPE_HASH. func_checker::types_compatible leads to type_compatible_p which will eventually do /* If we know the canonical types, compare them. */ if (TYPE_CANONICAL (inner_type) TYPE_CANONICAL (inner_type) == TYPE_CANONICAL (outer_type)) return true; and thus we will hapilly merge types with different TYPE_ALIGN. Should func_checker::types_compatible be extended to compare these? Clearly TYPE_ALIGN matters for vectorizer and other plaes... Any chance -malign-double can work and mix with -mno-align-double? I think we will mix the alignments because double is one of nodes we do not stream, right? Honza
Re: [PATCH] ICF: move readonly decision for variables to the right place
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index 864a5d0..5b1fcff 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -1410,10 +1410,6 @@ sem_variable::parse (varpool_node *node,
bitmap_obstack *stack)
if (node-alias)
return NULL;
- bool readonly = TYPE_P (decl) ? TYPE_READONLY (decl) : TREE_READONLY
(decl);
- if (!readonly)
-return NULL;
-
bool can_handle = DECL_VIRTUAL_P (decl)
|| flag_merge_constants = 2
|| (!TREE_ADDRESSABLE (decl) !node-externally_visible);
Please also drop can_handle and DECL_EXTERNAL check here.
can_handle seems to test if address matters. We already verify it at merging
time
and at a time we consider REF_ADDR references. This is enough.
Moreover TREE_ADDRESSABLE and externally_visible flags are subject to change in
between
analysis and WPA time.
Instead of this punt if THIS_VOLATILE is true.
Instead of ctor_for_folding bellow use DECL_INITIAL (ctor_for_folding may return
you NULL if it thinks the var is not safe)
@@ -1900,7 +1896,14 @@ sem_item_optimizer::filter_removed_items (void)
if (!flag_ipa_icf_variables)
remove_item (item);
else
- filtered.safe_push (item);
+ {
+ /* Filter out non-readonly variables. */
+ tree decl = item-decl;
+ if (TYPE_P (decl) ? TYPE_READONLY (decl) : TREE_READONLY (decl))
Just test TREE_READONLY (decl).
OK with these changes.
Honza
Re: ipa-icf::merge TLC
Hi Honza, This is more a note for other interested AArch64 testers, but this patch breaks some tests on aarch64-none-elf. Looking at the test output, this is a problem with the tests than with your patch. We now eliminate some function bodies which are identical across test functions, causing us to fail some scan-assembler directives. The fix is straightforward, we just need to add -fno-ipa-icf to get things working again. This keeps the spirit of the original tests. I'll propose a patch along those lines on Monday. Thanks, James --- PASS-FAIL: gcc.target/aarch64/atomic-comp-swap-release-acquire.c scan-assembler-times ldaxr\tw[0-9]+, \\[x[0-9]+\\] 4 PASS-FAIL: gcc.target/aarch64/atomic-comp-swap-release-acquire.c scan-assembler-times stlxr\tw[0-9]+, w[0-9]+, \\[x[0-9]+\\] 4 PASS-FAIL: gcc.target/aarch64/vect_saddl_1.c scan-assembler-times saddl2\tv[0-9]+.2d, v[0-9]+.4s, v[0-9]+.4s 2 PASS-FAIL: gcc.target/aarch64/vect_saddl_1.c scan-assembler-times saddl\tv[0-9]+.2d, v[0-9]+.2s, v[0-9]+.2s 2 PASS-FAIL: gcc.target/aarch64/vect_saddl_1.c scan-assembler-times ssubl2\tv[0-9]+.2d, v[0-9]+.4s, v[0-9]+.4s 5 PASS-FAIL: gcc.target/aarch64/vect_saddl_1.c scan-assembler-times ssubl\tv[0-9]+.2d, v[0-9]+.2s, v[0-9]+.2s 5 PASS-FAIL: gcc.target/aarch64/vect_smlal_1.c scan-assembler-times smlsl2\tv[0-9]+.8h 5 PASS-FAIL: gcc.target/aarch64/vect_smlal_1.c scan-assembler-times smlsl\tv[0-9]+.8h 5 Thanks, yes adding -fno-ipa-icf is a proper fix for tests like this. What happened was a logic bug in old implementation of merge routine. When symbol is externally visible it decided to create a wrapper (to preserve potential address compares) and to avoid wrapper cost redirect all direct uses. There was extra else between redirection and thunk creation, so often it redirected calls but it kept old function body in code. We now see a lot more DCE tham before. i386 tests also needed a compensation at two places. Honza
Re: [PATCH] ICF is more strict about non-common function and var, attributes.
On 02/28/2015 11:42 PM, Jan Hubicka wrote:
2015-02-28 Martin Liska mli...@suse.cz
Jan Hubicka hubi...@ucw.cz
* ipa-icf-gimple.c (func_checker::compare_variable_decl):
Validate variable alignment.
* ipa-icf.c (sem_function::equals_private): Be more precise
about non-common function attributes.
(sem_variable::equals): Likewise.
OK,
does this fix
gcc.target/i386/stackalign/longlong-2.c -mno-stackrealign
scan-assembler-times and[lq]?[^\\n]*-16,[^\\n]*sp 2
FAIL: gcc.target/i386/stackalign/longlong-2.c -mno-stackrealign
scan-assembler-times and[lq]?[^\\n]*-8,[^\\n]*sp 2
FAIL: gcc.target/i386/stackalign/longlong-2.c -mstackrealign
scan-assembler-times and[lq]?[^\\n]*-16,[^\\n]*sp 2
FAIL: gcc.target/i386/stackalign/longlong-2.c -mstackrealign
scan-assembler-times and[lq]?[^\\n]*-8,[^\\n]*sp 2
on -m32 mentioned in PR65237?
Honza
Half of FAILs are gone, but the rest is correctly merged (alignment matches).
Thus I would omit ICF
in this testcase. I'm going to install the patch.
Thanks,
Martin
From 60c5fdc5d5bab2d26a43813ffebda247c8dd1fce Mon Sep 17 00:00:00 2001
From: mliska mli...@suse.cz
Date: Fri, 27 Feb 2015 21:49:46 +0100
Subject: [PATCH 1/4] ICF is more strict about non-common function and var
attributes.
gcc/ChangeLog:
2015-02-28 Martin Liska mli...@suse.cz
Jan Hubicka hubi...@ucw.cz
* ipa-icf-gimple.c (func_checker::compare_variable_decl):
Validate variable alignment.
* ipa-icf.c (sem_function::equals_private): Be more precise
about non-common function attributes.
(sem_variable::equals): Likewise.
gcc/testsuite/ChangeLog:
2015-03-01 Martin Liska mli...@suse.cz
Jan Hubicka hubi...@ucw.cz
* gcc.target/i386/stackalign/longlong-2.c: Omit ICF.
---
gcc/ipa-icf-gimple.c | 3 ++
gcc/ipa-icf.c | 35 ++
.../gcc.target/i386/stackalign/longlong-2.c| 2 +-
3 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c
index 53d2c38..cbeb795 100644
--- a/gcc/ipa-icf-gimple.c
+++ b/gcc/ipa-icf-gimple.c
@@ -575,6 +575,9 @@ func_checker::compare_variable_decl (tree t1, tree t2)
if (t1 == t2)
return true;
+ if (DECL_ALIGN (t1) != DECL_ALIGN (t2))
+return return_false_with_msg (alignments are different);
+
if (DECL_HARD_REGISTER (t1) != DECL_HARD_REGISTER (t2))
return return_false_with_msg (DECL_HARD_REGISTER are different);
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index 5d50b6f..92133fc 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -619,6 +619,30 @@ sem_function::equals_private (sem_item *item,
if (!compare_phi_node (bb_sorted[i]-bb, m_compared_func-bb_sorted[i]-bb))
return return_false_with_msg (PHI node comparison returns false);
+ /* Compare special function DECL attributes. */
+ if (DECL_FUNCTION_PERSONALITY (decl) != DECL_FUNCTION_PERSONALITY (item-decl))
+return return_false_with_msg (function personalities are different);
+
+ if (DECL_DISREGARD_INLINE_LIMITS (decl) != DECL_DISREGARD_INLINE_LIMITS (item-decl))
+return return_false_with_msg (DECL_DISREGARD_INLINE_LIMITS are different);
+
+ if (DECL_DECLARED_INLINE_P (decl) != DECL_DECLARED_INLINE_P (item-decl))
+return return_false_with_msg (inline attributes are different);
+
+ if (DECL_IS_OPERATOR_NEW (decl) != DECL_IS_OPERATOR_NEW (item-decl))
+return return_false_with_msg (operator new flags are different);
+
+ if (DECL_NO_INSTRUMENT_FUNCTION_ENTRY_EXIT (decl)
+ != DECL_NO_INSTRUMENT_FUNCTION_ENTRY_EXIT (item-decl))
+return return_false_with_msg (intrument function entry exit
+ attributes are different);
+
+ if (DECL_NO_LIMIT_STACK (decl) != DECL_NO_LIMIT_STACK (item-decl))
+return return_false_with_msg (no stack limit attributes are different);
+
+ if (flags_from_decl_or_type (decl) != flags_from_decl_or_type (item-decl))
+return return_false_with_msg (decl_or_type flags are different);
+
return result;
}
@@ -1280,6 +1304,17 @@ sem_variable::equals (sem_item *item,
if (!ctor || !v-ctor)
return return_false_with_msg (ctor is missing for semantic variable);
+ if (DECL_IN_CONSTANT_POOL (decl)
+ (DECL_IN_CONSTANT_POOL (item-decl)
+ || item-node-address_matters_p ()))
+return return_false_with_msg (constant pool);
+
+ if (DECL_IN_TEXT_SECTION (decl) != DECL_IN_TEXT_SECTION (item-decl))
+return return_false_with_msg (text section);
+
+ if (DECL_TLS_MODEL (decl) || DECL_TLS_MODEL (item-decl))
+return return_false_with_msg (TLS model);
+
return sem_variable::equals (ctor, v-ctor);
}
diff --git a/gcc/testsuite/gcc.target/i386/stackalign/longlong-2.c b/gcc/testsuite/gcc.target/i386/stackalign/longlong-2.c
index 6ea83f9..d52b9d1 100644
--- a/gcc/testsuite/gcc.target/i386/stackalign/longlong-2.c
+++ b/gcc/testsuite/gcc.target/i386/stackalign/longlong-2.c
@@ -1,6 +1,6 @@
/* { dg-do compile {
Prevent merging across COMDAT group boundary
Hi,
tihs patch fixes one case I forgot in sem_function::merge: When target is not
in comdat group, we still can not make alias in the case source is.
Bootstrapped/regtested x86_64-linux, Martin, can you double check this fixes
the xalancbmk?
Honza
PR ipa/65237
* ipa-icf.c (sem_function::merge): Do not attempt to produce alias
across COMDAT group boundary.
Index: ipa-icf.c
===
--- ipa-icf.c (revision 221076)
+++ ipa-icf.c (working copy)
@@ -745,12 +755,17 @@ sem_function::merge (sem_item *alias_ite
it is an external functions where we can not create an alias
(ORIGINAL_DISCARDABLE)
3) if target do not support symbol aliases.
+ 4) original and alias lie in different comdat groups.
If we can not produce alias, we will turn ALIAS into WRAPPER of ORIGINAL
and/or redirect all callers from ALIAS to ORIGINAL. */
if ((original_address_matters alias_address_matters)
- || original_discardable
- || !sem_item::target_supports_symbol_aliases_p ())
+ || (original_discardable
+ (!DECL_COMDAT_GROUP (alias-decl)
+ || (DECL_COMDAT_GROUP (alias-decl)
+ != DECL_COMDAT_GROUP (original-decl
+ || !sem_item::target_supports_symbol_aliases_p ()
+ || DECL_COMDAT_GROUP (alias-decl) != DECL_COMDAT_GROUP (original-decl))
{
/* First see if we can produce wrapper. */
[PATCH] ICF is more strict about non-common function and var, attributes.
Hello. Following patch is guard for special non-common function attributes that can be spotted by ICF machinery. Tested on x86_64-linux. Ready for trunk? Thanks, Martin From 13e8c55b6317bf4e2c21ef04305cb3a20ad51255 Mon Sep 17 00:00:00 2001 From: mliska mli...@suse.cz Date: Fri, 27 Feb 2015 21:49:46 +0100 Subject: [PATCH 1/4] ICF is more strict about non-common function and var attributes. gcc/ChangeLog: 2015-02-28 Martin Liska mli...@suse.cz Jan Hubicka hubi...@ucw.cz * ipa-icf-gimple.c (func_checker::compare_variable_decl): Validate variable alignment. * ipa-icf.c (sem_function::equals_private): Be more precise about non-common function attributes. (sem_variable::equals): Likewise. --- gcc/ipa-icf-gimple.c | 3 +++ gcc/ipa-icf.c| 35 +++ 2 files changed, 38 insertions(+) diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c index 53d2c38..cbeb795 100644 --- a/gcc/ipa-icf-gimple.c +++ b/gcc/ipa-icf-gimple.c @@ -575,6 +575,9 @@ func_checker::compare_variable_decl (tree t1, tree t2) if (t1 == t2) return true; + if (DECL_ALIGN (t1) != DECL_ALIGN (t2)) +return return_false_with_msg (alignments are different); + if (DECL_HARD_REGISTER (t1) != DECL_HARD_REGISTER (t2)) return return_false_with_msg (DECL_HARD_REGISTER are different); diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c index 5d50b6f..92133fc 100644 --- a/gcc/ipa-icf.c +++ b/gcc/ipa-icf.c @@ -619,6 +619,30 @@ sem_function::equals_private (sem_item *item, if (!compare_phi_node (bb_sorted[i]-bb, m_compared_func-bb_sorted[i]-bb)) return return_false_with_msg (PHI node comparison returns false); + /* Compare special function DECL attributes. */ + if (DECL_FUNCTION_PERSONALITY (decl) != DECL_FUNCTION_PERSONALITY (item-decl)) +return return_false_with_msg (function personalities are different); + + if (DECL_DISREGARD_INLINE_LIMITS (decl) != DECL_DISREGARD_INLINE_LIMITS (item-decl)) +return return_false_with_msg (DECL_DISREGARD_INLINE_LIMITS are different); + + if (DECL_DECLARED_INLINE_P (decl) != DECL_DECLARED_INLINE_P (item-decl)) +return return_false_with_msg (inline attributes are different); + + if (DECL_IS_OPERATOR_NEW (decl) != DECL_IS_OPERATOR_NEW (item-decl)) +return return_false_with_msg (operator new flags are different); + + if (DECL_NO_INSTRUMENT_FUNCTION_ENTRY_EXIT (decl) + != DECL_NO_INSTRUMENT_FUNCTION_ENTRY_EXIT (item-decl)) +return return_false_with_msg (intrument function entry exit + attributes are different); + + if (DECL_NO_LIMIT_STACK (decl) != DECL_NO_LIMIT_STACK (item-decl)) +return return_false_with_msg (no stack limit attributes are different); + + if (flags_from_decl_or_type (decl) != flags_from_decl_or_type (item-decl)) +return return_false_with_msg (decl_or_type flags are different); + return result; } @@ -1280,6 +1304,17 @@ sem_variable::equals (sem_item *item, if (!ctor || !v-ctor) return return_false_with_msg (ctor is missing for semantic variable); + if (DECL_IN_CONSTANT_POOL (decl) + (DECL_IN_CONSTANT_POOL (item-decl) + || item-node-address_matters_p ())) +return return_false_with_msg (constant pool); + + if (DECL_IN_TEXT_SECTION (decl) != DECL_IN_TEXT_SECTION (item-decl)) +return return_false_with_msg (text section); + + if (DECL_TLS_MODEL (decl) || DECL_TLS_MODEL (item-decl)) +return return_false_with_msg (TLS model); + return sem_variable::equals (ctor, v-ctor); } -- 2.1.2
[PATCH] Fix for PR ipa/65245
Hello.
Following patch skips all variable aliases as potential merge candidates.
Tested on x86_64-linux.
Ready for trunk?
Thanks,
Martin
From fbde2e98f98a71105d18cf3e91e8032d0c657139 Mon Sep 17 00:00:00 2001
From: mliska mli...@suse.cz
Date: Fri, 27 Feb 2015 22:42:49 +0100
Subject: [PATCH 2/4] ICF: Do not consider variable aliases for merge
operation.
gcc/ChangeLog:
2015-02-28 Martin Liska mli...@suse.cz
Jan Hubicka hubi...@ucw.cz
PR ipa/65245
* ipa-icf.c (sem_function::parse): Do not consider aliases.
(sem_variable::parse): Likewise.
(sem_item_optimizer::build_graph): Consider ultimate aliases
for references.
---
gcc/ipa-icf.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index 92133fc..864a5d0 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -1095,7 +1095,7 @@ sem_function::parse (cgraph_node *node, bitmap_obstack *stack)
tree fndecl = node-decl;
function *func = DECL_STRUCT_FUNCTION (fndecl);
- /* TODO: add support for thunks and aliases. */
+ /* TODO: add support for thunks. */
if (!func || !node-has_gimple_body_p ())
return NULL;
@@ -1407,6 +1407,9 @@ sem_variable::parse (varpool_node *node, bitmap_obstack *stack)
{
tree decl = node-decl;
+ if (node-alias)
+return NULL;
+
bool readonly = TYPE_P (decl) ? TYPE_READONLY (decl) : TREE_READONLY (decl);
if (!readonly)
return NULL;
@@ -2057,7 +2060,8 @@ sem_item_optimizer::build_graph (void)
cgraph_edge *e = cnode-callees;
while (e)
{
- sem_item **slot = m_symtab_node_map.get (e-callee);
+ sem_item **slot = m_symtab_node_map.get
+ (e-callee-ultimate_alias_target ());
if (slot)
item-add_reference (*slot);
@@ -2068,7 +2072,8 @@ sem_item_optimizer::build_graph (void)
ipa_ref *ref = NULL;
for (unsigned i = 0; item-node-iterate_reference (i, ref); i++)
{
- sem_item **slot = m_symtab_node_map.get (ref-referred);
+ sem_item **slot = m_symtab_node_map.get
+ (ref-referred-ultimate_alias_target ());
if (slot)
item-add_reference (*slot);
}
--
2.1.2
Re: [PATCH] ICF is more strict about non-common function and var, attributes.
Half of FAILs are gone, but the rest is correctly merged (alignment matches). Thus I would omit ICF in this testcase. I'm going to install the patch. Thanks, Martin From 60c5fdc5d5bab2d26a43813ffebda247c8dd1fce Mon Sep 17 00:00:00 2001 From: mliska mli...@suse.cz Date: Fri, 27 Feb 2015 21:49:46 +0100 Subject: [PATCH 1/4] ICF is more strict about non-common function and var attributes. gcc/ChangeLog: 2015-02-28 Martin Liska mli...@suse.cz Jan Hubicka hubi...@ucw.cz * ipa-icf-gimple.c (func_checker::compare_variable_decl): Validate variable alignment. * ipa-icf.c (sem_function::equals_private): Be more precise about non-common function attributes. (sem_variable::equals): Likewise. gcc/testsuite/ChangeLog: 2015-03-01 Martin Liska mli...@suse.cz Jan Hubicka hubi...@ucw.cz * gcc.target/i386/stackalign/longlong-2.c: Omit ICF. OK then. Thanks! Are we ready to close the PR? Honza
Re: [patch, avr] Take 2: Fix PR64331: insn output and insn length computation rely on REG_DEAD notes.
Am 02/26/2015 um 11:45 PM schrieb Steven Bosscher: On Thu, Feb 26, 2015 at 8:35 PM, Georg-Johann Lay wrote: Take #2 introduces a new, avr-specific rtl pass whose sole purpose is to rectify notes. The pass is scheduled right before cfg does down (right before .*free_cfg) so that cfg and hence df machinery is available. Regression tests look fine and for the test case the patches produce correct code and correct insn length. Sorry for party-pooping, but it seems to me that the real bug is that the target is lying to reload. IIUC the AVR port selects the insn alternative after register allocation (after reload). It bases its selection on REG_DEAD notes. In PR64331 an alternative is used that clobbers r20 that has a REG_DEAD note, but r20 is not actually dead because hardreg-cprop has propagated it forward without adjusting the note. It's not actually about constraint alternatives. Let me give an example: Testing HI for 0. The usual sequence would be cc0 = reg.low == 0 cc0 = cc0 reg.high == 0 which costs 2 instructions. If reg is unused after, then ORing can be used and cc0 will be set as a side effect. This costs 1 insn: cc0 = (reg.low |= reg.high) Using alternatives would double their number, i.e. 14 instead of 7 for *cmphi. These constraint alternatives would have to express 1) reg-alloc, please, use alternative #1 (with clobber of reg) if the register is unused after 2) reg-alloc, please, use alternative #2 (not clobbering reg) if the register is used after If we assume for a moment that we have a *testhi insn and the old *tsthi had just 1 alternative: (define_insn *tsthi [(set (cc0) (compare (match_operand:ALL2 0 register_operand r) (const_int 0)))] ...) Then the new one would be something like (define_insn *tsthi [(set (cc0) (compare (match_operand:ALL2 0 register_operand r,r) (const_int 0))) (clobber (match_scratch:HI 1 =0,X)] ...) But how can I express 1) and 2) ? reg-alloc's choices appear to be random from BE's perspective, and it fails with much simpler tasks to produce good results. The first alternative which matches is the 1st, i.e. it would always clobber op 0 which results in up to 4 instructions and higher register pressure if that register is still needed after the insn. Making alternative 1 more expensive presumably results always in 2nd alternateive, i.e. 2 instructions instead of 1 if op 0 is no more needed. Similar if we swap alternatives #1 and #2: Then r,X will always match and appear to be cheaper than r,=0. Currently, my preferred approach is a new drop-in replacement for the old reg_unused_after which uses clobbers to decide whether or not op 0 is still needed. That way, reg-alloc can work like before and there is no need to implement dozens of new constraint alternatives across the md files. The clobbers, in turn, are added by a target-pass which translates deadness information to clobbers. The preferred placement of that pass is the same as for the proposed new avr pass from patch take #2. Below you find a quick hack that outlines the idea: New insn attribute dead tags insns which are worth to be taken into account for such dead--clobber translation and provide the operand number. The new pass scans all insns and translates dead info to clobbers as requested. This needs much more work, of course, like adjusting peepholes and splitters to the new insns with their additional operand. avr_reg_unused_after_df (the old reg_unused_after) should be rewritten to use DF info instead of scanning by hand. Some of the insn templates which used reg_unused_after are move insns; for these insns it's a bit different. The normal way of things is that the insn alternative is selected in reload (or in LRA) and that the clobbers are added as necessary. In Would you outline a concrete example? In particular, how to avoid clobbers if the operand is no more needed after the insn, and use clobber alternative if the value is no more needed? I.e. how would you express 1) and 2) from above in terms of insn constraints? PR64331, an alternative for insn r17 would be selected that has a CLOBBER for r20, prevent hardreg-cprop from propagating r20. Selecting insns based on REG-notes is dangerous business. Lying to reload and to post-RA passes is a mortal sin, the compiler will punish you. There is no guarantee that nothing will change between your new pass to recompute notes, and the final pass that emits the insns. Ya. However, for 4.9 I'd still propose patch take #2. It definitely improves matters, does nothing wrong, and throwing out reg_unused_after completely will lead to considerable increase of code size and run time. And for 4.9 a big change like outlined above is way too intrusive for already released version, imo. For 5.0 I am also still proposing the patchh. It does nothing wrong and is the base for a complete
[RFC 1/2] Turn RETURN_ADDR_IN_PREVIOUS_FRAME into C expression
This allows a target to support both windowed and non-windowed ABI.
2015-02-28 Max Filippov jcmvb...@gmail.com
gcc/
* builtins.c (expand_builtin_return_addr): Add
RETURN_ADDR_IN_PREVIOUS_FRAME to 'if' condition.
* config/sparc/sparc.h (RETURN_ADDR_IN_PREVIOUS_FRAME): Change
definition to 1.
* config/xtensa/xtensa.h (RETURN_ADDR_IN_PREVIOUS_FRAME):
Likewise.
---
gcc/builtins.c | 2 +-
gcc/config/sparc/sparc.h | 2 +-
gcc/config/xtensa/xtensa.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/gcc/builtins.c b/gcc/builtins.c
index fb871e6..0cc0c68 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -803,7 +803,7 @@ expand_builtin_return_addr (enum built_in_function
fndecl_code, int count)
pointer, but it can be accessed off the previous frame pointer by
reading the value from the register window save area. */
#ifdef RETURN_ADDR_IN_PREVIOUS_FRAME
- if (fndecl_code == BUILT_IN_RETURN_ADDRESS)
+ if (RETURN_ADDR_IN_PREVIOUS_FRAME fndecl_code == BUILT_IN_RETURN_ADDRESS)
count--;
#endif
diff --git a/gcc/config/sparc/sparc.h b/gcc/config/sparc/sparc.h
index c6100a1..27ad748 100644
--- a/gcc/config/sparc/sparc.h
+++ b/gcc/config/sparc/sparc.h
@@ -1293,7 +1293,7 @@ do {
\
access it from the current frame pointer. We can access it from the
previous frame pointer though by reading a value from the register window
save area. */
-#define RETURN_ADDR_IN_PREVIOUS_FRAME
+#define RETURN_ADDR_IN_PREVIOUS_FRAME 1
/* This is the offset of the return address to the true next instruction to be
executed for the current function. */
diff --git a/gcc/config/xtensa/xtensa.h b/gcc/config/xtensa/xtensa.h
index 74ca240..14fe4bb 100644
--- a/gcc/config/xtensa/xtensa.h
+++ b/gcc/config/xtensa/xtensa.h
@@ -615,7 +615,7 @@ typedef struct xtensa_args
/* Define this if the return address of a particular stack frame is
accessed from the frame pointer of the previous stack frame. */
-#define RETURN_ADDR_IN_PREVIOUS_FRAME
+#define RETURN_ADDR_IN_PREVIOUS_FRAME 1
/* A C expression whose value is RTL representing the value of the
return address for the frame COUNT steps up from the current
--
1.8.1.4
[RFC 0/2] xtensa: add call0 ABI support
Hello, this series adds call0 ABI support for xtensa. Code generation part is well tested, but I'm not 100% sure about the exception handling part. Please review. Max Filippov (2): Turn RETURN_ADDR_IN_PREVIOUS_FRAME into C expression Implement call0 ABI for xtensa gcc/builtins.c | 2 +- gcc/config/sparc/sparc.h| 2 +- gcc/config/xtensa/constraints.md| 6 +- gcc/config/xtensa/xtensa-protos.h | 3 + gcc/config/xtensa/xtensa.c | 561 gcc/config/xtensa/xtensa.h | 67 +++-- gcc/config/xtensa/xtensa.md | 48 ++- libgcc/config/xtensa/lib2funcs.S| 10 + libgcc/config/xtensa/linux-unwind.h | 3 + libgcc/config/xtensa/t-windowed | 2 + libgcc/config/xtensa/t-xtensa | 3 - libgcc/configure| 21 ++ libgcc/configure.ac | 16 + 13 files changed, 588 insertions(+), 156 deletions(-) create mode 100644 libgcc/config/xtensa/t-windowed -- 1.8.1.4
Fix thunk generation for functions returning DECL_BY_REFERENCE
Hi,
the Chromium miscompilation is caused by introducing extra copy when returning
non-POD object (as tested in the testcase). This is bug in
cgraph_node::expand_thunk not setting gimple_call_set_return_slot_opt (I copied
same code from ipa-split).
This bug should reproduce on all release branches for target not defining
assembler thunks.
Bootstrapped/regtested x86_64-linux, comitted, earlier version was tested by
Martin to fix chromium.
Honza
PR ipa/65236
* g++.dg/ipa/ipa-icf-6.C: New testcase.
* cgraphunit.c (cgraph_node::expand_thunk): Enable return slot
opt.
Index: testsuite/g++.dg/ipa/ipa-icf-6.C
===
--- testsuite/g++.dg/ipa/ipa-icf-6.C(revision 0)
+++ testsuite/g++.dg/ipa/ipa-icf-6.C(revision 0)
@@ -0,0 +1,37 @@
+/* { dg-do run } */
+/* { dg-options -O3 -fdump-ipa-icf } */
+
+struct A {
+ A() {ptr=b;}
+ A(const A a) {ptr = b;}
+ void test() { if (ptr != b) __builtin_abort ();}
+ int b;
+ int *ptr;
+};
+
+A test1(A a)
+{
+ a.test();
+ return a;
+}
+A test2(A a)
+{
+ a.test();
+ return a;
+}
+__attribute__ ((noinline))
+static void
+test_me (A (*t)(A))
+{
+ struct A a, b=t(a);
+ b.test ();
+}
+int
+main()
+{
+ test_me (test1);
+ test_me (test2);
+ return 0;
+}
+/* { dg-final { scan-ipa-dump-times Unified; Wrapper has been created 1
icf } } */
+/* { dg-final { cleanup-ipa-dump icf } } */
Index: cgraphunit.c
===
--- cgraphunit.c(revision 221076)
+++ cgraphunit.c(working copy)
@@ -1680,6 +1680,14 @@ cgraph_node::expand_thunk (bool output_a
callees-call_stmt = call;
gimple_call_set_from_thunk (call, true);
gimple_call_set_with_bounds (call, instrumentation_clone);
+
+ /* Return slot optimization is always possible and in fact requred to
+ return values with DECL_BY_REFERENCE. */
+ if (aggregate_value_p (resdecl, TREE_TYPE (thunk_fndecl))
+ (!is_gimple_reg_type (TREE_TYPE (resdecl))
+ || DECL_BY_REFERENCE (resdecl)))
+gimple_call_set_return_slot_opt (call, true);
+
if (restmp !alias_is_noreturn)
{
gimple_call_set_lhs (call, restmp);
[PATCH] PR target/65248: [5 Regression] Copy relocation in PIE against protected symbol
Ue copy relocation in PIE improves performance. But copy relocation
can't be used to access protected symbols defined in shared libaries
and linker in binutils 2.26 enforces doesn't allow it. GCC doesn't
know if an external definition is protected or not. This option adds
-mcopyreloc-in-pie to give user an option to turn it off to avoid problem
at link-time. OK for trunk?
Thanks.
H.J.
---
gcc/
PR target/65248
* config/i386/i386.c (ix86_option_override_internal): Set
flag_copyreloc_in_pie to HAVE_LD_PIE_COPYRELOC if not set.
(legitimate_pic_address_disp_p): Replace HAVE_LD_PIE_COPYRELOC
with flag_copyreloc_in_pie.
* config/i386/i386.opt (mcopyreloc-in-pie): New.
* doc/invoke.texi: Document -mcopyreloc-in-pie.
gcc/testsuite/
PR target/65248
* gcc.target/i386/pr65248-1.c: New.
* gcc.target/i386/pr65248-2.c: Likewise.
* gcc.target/i386/pr65248-3.c: Likewise.
* gcc.target/i386/pr65248-4.c: Likewise.
---
gcc/config/i386/i386.c| 6 +-
gcc/config/i386/i386.opt | 4
gcc/doc/invoke.texi | 10 +-
gcc/testsuite/gcc.target/i386/pr65248-1.c | 13 +
gcc/testsuite/gcc.target/i386/pr65248-2.c | 13 +
gcc/testsuite/gcc.target/i386/pr65248-3.c | 16
gcc/testsuite/gcc.target/i386/pr65248-4.c | 16
7 files changed, 76 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/i386/pr65248-1.c
create mode 100644 gcc/testsuite/gcc.target/i386/pr65248-2.c
create mode 100644 gcc/testsuite/gcc.target/i386/pr65248-3.c
create mode 100644 gcc/testsuite/gcc.target/i386/pr65248-4.c
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index bec1324..6768ee8 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -4248,6 +4248,10 @@ ix86_option_override_internal (bool main_args_p,
#endif
}
+ /* Set the default value for -mcopyreloc-in-pie. */
+ if (opts-x_flag_copyreloc_in_pie == -1)
+opts-x_flag_copyreloc_in_pie = HAVE_LD_PIE_COPYRELOC;
+
if (!(opts_set-x_target_flags MASK_VZEROUPPER))
opts-x_target_flags |= MASK_VZEROUPPER;
if (!ix86_tune_features[X86_TUNE_AVX256_UNALIGNED_LOAD_OPTIMAL]
@@ -13230,7 +13234,7 @@ legitimate_pic_address_disp_p (rtx disp)
}
else if (!SYMBOL_REF_FAR_ADDR_P (op0)
(SYMBOL_REF_LOCAL_P (op0)
- || (HAVE_LD_PIE_COPYRELOC
+ || (flag_copyreloc_in_pie
flag_pie
!SYMBOL_REF_WEAK (op0)
!SYMBOL_REF_FUNCTION_P (op0)))
diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt
index 301430c..55c712c 100644
--- a/gcc/config/i386/i386.opt
+++ b/gcc/config/i386/i386.opt
@@ -826,6 +826,10 @@ mfentry
Target Report Var(flag_fentry) Init(-1)
Emit profiling counter call at function entry before prologue.
+mcopyreloc-in-pie
+Target Report Var(flag_copyreloc_in_pie) Init(-1)
+Use copy relocations in Position Independent Executable (PIE)
+
mrecord-mcount
Target Report Var(flag_record_mcount) Init(0)
Generate __mcount_loc section with all mcount or __fentry__ calls.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index a87376e..7fd4e77 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -1078,7 +1078,8 @@ See RS/6000 and PowerPC Options.
-m32 -m64 -mx32 -m16 -mlarge-data-threshold=@var{num} @gol
-msse2avx -mfentry -mrecord-mcount -mnop-mcount -m8bit-idiv @gol
-mavx256-split-unaligned-load -mavx256-split-unaligned-store @gol
--malign-data=@var{type} -mstack-protector-guard=@var{guard}}
+-malign-data=@var{type} -mstack-protector-guard=@var{guard}} @gol
+-mcopyreloc-in-pie
@emph{x86 Windows Options}
@gccoptlist{-mconsole -mcygwin -mno-cygwin -mdll @gol
@@ -22704,6 +22705,13 @@ For systems that use the GNU C Library, the default is
on.
Specify that the assembler should encode SSE instructions with VEX
prefix. The option @option{-mavx} turns this on by default.
+@item -mcopyreloc-in-pie
+@itemx -mno-copyreloc-in-pie
+@opindex mcopyreloc-in-pie
+Use copy relocations in Position Independent Executable (PIE). It
+requires linker support. This option is turned on by default if linker
+used to build GCC supports it.
+
@item -mfentry
@itemx -mno-fentry
@opindex mfentry
diff --git a/gcc/testsuite/gcc.target/i386/pr65248-1.c
b/gcc/testsuite/gcc.target/i386/pr65248-1.c
new file mode 100644
index 000..91aa6be
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr65248-1.c
@@ -0,0 +1,13 @@
+/* Check that GOTPCREL isn't used to access glob_a. */
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options -O2 -fpie -mcopyreloc-in-pie } */
+
+extern int glob_a;
+
+int foo ()
+{
+ return glob_a;
+}
+
+/* glob_a should never be accessed with a GOTPCREL. */
+/* { dg-final { scan-assembler-not glob_a@GOTPCREL { target { ! ia32 } } }
Re: [PATCH] PR target/65248: [5 Regression] Copy relocation in PIE against protected symbol
On 02/28/2015 09:42 AM, H.J. Lu wrote:
@@ -22704,6 +22705,13 @@ For systems that use the GNU C Library, the default is
on.
Specify that the assembler should encode SSE instructions with VEX
prefix. The option @option{-mavx} turns this on by default.
+@item -mcopyreloc-in-pie
+@itemx -mno-copyreloc-in-pie
+@opindex mcopyreloc-in-pie
+Use copy relocations in Position Independent Executable (PIE). It
+requires linker support. This option is turned on by default if linker
+used to build GCC supports it.
+
How about: ...if GCC is configured to use a linker that supports these
relocations.
I assume this is a property of the target linker, and not literally the
host linker used to build GCC?
-Sandra
Re: [RFC 1/2] Turn RETURN_ADDR_IN_PREVIOUS_FRAME into C expression
On Sat, Feb 28, 2015 at 10:14 AM, Max Filippov jcmvb...@gmail.com wrote: This allows a target to support both windowed and non-windowed ABI. 2015-02-28 Max Filippov jcmvb...@gmail.com gcc/ * builtins.c (expand_builtin_return_addr): Add RETURN_ADDR_IN_PREVIOUS_FRAME to 'if' condition. * config/sparc/sparc.h (RETURN_ADDR_IN_PREVIOUS_FRAME): Change definition to 1. * config/xtensa/xtensa.h (RETURN_ADDR_IN_PREVIOUS_FRAME): Likewise. Unfortunately, I can't approve a change to spark.h, even though this seems quite innocuous to me. I'm not sure why it is even relevant to xtensa. Without that, approved.
Fix option andling in can_inline_edge_p
hi,
this patch fixes bug in can_inline_edge_p where opt_for_fn is taken from
inline clone and not function being inlined into itself. Those opt_for_fn
are going to be ignored and thus have no meaning.
I also fixed case wehre !flag_devirtualize would get inlined into
flag_devirtualize but at WPA time we possibly made changes clobbering type
information.
Bootstrapped/regtested x86_64-linux, will commit it shortly.
* ipa-inline.c (can_inline_edge_p): Match opt_for_fn on inline
target; also match flag_ipa_devirt.
Index: ipa-inline.c
===
--- ipa-inline.c(revision 221076)
+++ ipa-inline.c(working copy)
@@ -310,7 +310,7 @@ sanitize_attrs_match_for_inline_p (const
static bool
can_inline_edge_p (struct cgraph_edge *e, bool report,
- bool disregard_limits = false)
+ bool disregard_limits = false, bool early = false)
{
bool inlinable = true;
enum availability avail;
@@ -409,39 +409,48 @@ can_inline_edge_p (struct cgraph_edge *e
Not even for always_inline declared functions. */
/* Strictly speaking only when the callee contains signed integer
math where overflow is undefined. */
- if ((opt_for_fn (e-caller-decl, flag_strict_overflow)
- != opt_for_fn (e-caller-decl, flag_strict_overflow))
- || (opt_for_fn (e-caller-decl, flag_wrapv)
- != opt_for_fn (e-caller-decl, flag_wrapv))
- || (opt_for_fn (e-caller-decl, flag_trapv)
- != opt_for_fn (e-caller-decl, flag_trapv))
+ if ((opt_for_fn (caller-decl, flag_strict_overflow)
+ != opt_for_fn (caller-decl, flag_strict_overflow))
+ || (opt_for_fn (caller-decl, flag_wrapv)
+ != opt_for_fn (caller-decl, flag_wrapv))
+ || (opt_for_fn (caller-decl, flag_trapv)
+ != opt_for_fn (caller-decl, flag_trapv))
/* Strictly speaking only when the callee contains memory
accesses that are not using alias-set zero anyway. */
- || (opt_for_fn (e-caller-decl, flag_strict_aliasing)
- != opt_for_fn (e-caller-decl, flag_strict_aliasing))
+ || (opt_for_fn (caller-decl, flag_strict_aliasing)
+ != opt_for_fn (caller-decl, flag_strict_aliasing))
/* Strictly speaking only when the callee uses FP math. */
- || (opt_for_fn (e-caller-decl, flag_rounding_math)
- != opt_for_fn (e-caller-decl, flag_rounding_math))
- || (opt_for_fn (e-caller-decl, flag_trapping_math)
- != opt_for_fn (e-caller-decl, flag_trapping_math))
- || (opt_for_fn (e-caller-decl, flag_unsafe_math_optimizations)
- != opt_for_fn (e-caller-decl, flag_unsafe_math_optimizations))
- || (opt_for_fn (e-caller-decl, flag_finite_math_only)
- != opt_for_fn (e-caller-decl, flag_finite_math_only))
- || (opt_for_fn (e-caller-decl, flag_signaling_nans)
- != opt_for_fn (e-caller-decl, flag_signaling_nans))
- || (opt_for_fn (e-caller-decl, flag_cx_limited_range)
- != opt_for_fn (e-caller-decl, flag_cx_limited_range))
- || (opt_for_fn (e-caller-decl, flag_signed_zeros)
- != opt_for_fn (e-caller-decl, flag_signed_zeros))
- || (opt_for_fn (e-caller-decl, flag_associative_math)
- != opt_for_fn (e-caller-decl, flag_associative_math))
- || (opt_for_fn (e-caller-decl, flag_reciprocal_math)
- != opt_for_fn (e-caller-decl, flag_reciprocal_math))
+ || (opt_for_fn (caller-decl, flag_rounding_math)
+ != opt_for_fn (caller-decl, flag_rounding_math))
+ || (opt_for_fn (caller-decl, flag_trapping_math)
+ != opt_for_fn (caller-decl, flag_trapping_math))
+ || (opt_for_fn (caller-decl, flag_unsafe_math_optimizations)
+ != opt_for_fn (caller-decl, flag_unsafe_math_optimizations))
+ || (opt_for_fn (caller-decl, flag_finite_math_only)
+ != opt_for_fn (caller-decl, flag_finite_math_only))
+ || (opt_for_fn (caller-decl, flag_signaling_nans)
+ != opt_for_fn (caller-decl, flag_signaling_nans))
+ || (opt_for_fn (caller-decl, flag_cx_limited_range)
+ != opt_for_fn (caller-decl, flag_cx_limited_range))
+ || (opt_for_fn (caller-decl, flag_signed_zeros)
+ != opt_for_fn (caller-decl, flag_signed_zeros))
+ || (opt_for_fn (caller-decl, flag_associative_math)
+ != opt_for_fn (caller-decl, flag_associative_math))
+ || (opt_for_fn (caller-decl, flag_reciprocal_math)
+ != opt_for_fn (caller-decl, flag_reciprocal_math))
/* Strictly speaking only when the callee contains function
calls that may end up setting errno. */
- || (opt_for_fn (e-caller-decl, flag_errno_math)
- != opt_for_fn (e-caller-decl, flag_errno_math)))
+ || (opt_for_fn
Fix handling of OBJ_TYPE_REF in ipa-icf-gimple.c
Hi,
this patch fixes OBJ_TYPE_REF handling. We need to compare OTR type for match
and we can skip comparing when devirtualization is disabled.
Honza
* ipa-icf-gimple.c (func_checker::compare_operand): Fix handling
of OBJ_TYPE_REF.
Index: ipa-icf-gimple.c
===
--- ipa-icf-gimple.c(revision 221083)
+++ ipa-icf-gimple.c(working copy)
@@ -448,18 +448,23 @@ func_checker::compare_operand (tree t1,
/* Virtual table call. */
case OBJ_TYPE_REF:
{
- x1 = TREE_OPERAND (t1, 0);
- x2 = TREE_OPERAND (t2, 0);
- y1 = TREE_OPERAND (t1, 1);
- y2 = TREE_OPERAND (t2, 1);
- z1 = TREE_OPERAND (t1, 2);
- z2 = TREE_OPERAND (t2, 2);
+ if (!compare_ssa_name (OBJ_TYPE_REF_EXPR (t1), OBJ_TYPE_REF_EXPR (t2)))
+ return return_false ();
+ if (opt_for_fn (m_source_func_decl, flag_devirtualize)
+virtual_method_call_p (t1))
+ {
+ if (tree_to_uhwi (OBJ_TYPE_REF_TOKEN (t1))
+ != tree_to_uhwi (OBJ_TYPE_REF_TOKEN (t2)))
+ return return_false_with_msg (OBJ_TYPE_REF token mismatch);
+ if (!types_same_for_odr (obj_type_ref_class (t1),
+obj_type_ref_class (t2)))
+ return return_false_with_msg (OBJ_TYPE_REF OTR type mismatch);
+ if (!compare_ssa_name (OBJ_TYPE_REF_OBJECT (t1),
+ OBJ_TYPE_REF_OBJECT (t2)))
+ return return_false_with_msg (OBJ_TYPE_REF object mismatch);
+ }
- ret = compare_ssa_name (x1, x2)
- compare_operand (y1, y2)
- compare_cst_or_decl (z1, z2);
-
- return return_with_debug (ret);
+ return return_with_debug (true);
}
case IMAGPART_EXPR:
case REALPART_EXPR:
More of icf::merge TLC
Hi,
the prevoius bug in COMDAT handling made me to revisit the code once again.
There was few bugs fixed bellow.
1) If we have resolution info and we know original is going to be discarded
we need to prevail COMDAT logic
2) It is possible to merge non-comdat to comdat and comdat to non-comdat.
It is just not possible to merge two comdats to avoid possible cycles.
3) We do not want to rpoduce local aliases inside comdats since these will be
comdat locals and prevent inlining.
Bootstrapped/regtested x86_64-linux, commited.
Honza
* ipa-icf.c (sem_function::merge): Fix handling of COMDAT.
(sem_variable::merge) Likewise.
Index: ipa-icf.c
===
--- ipa-icf.c (revision 221079)
+++ ipa-icf.c (working copy)
@@ -709,6 +709,7 @@ sem_function::merge (sem_item *alias_ite
bool remove = false;
bool original_discardable = false;
+ bool original_discarded = false;
bool original_address_matters = original-address_matters_p ();
bool alias_address_matters = alias-address_matters_p ();
@@ -737,15 +738,16 @@ sem_function::merge (sem_item *alias_ite
}
/* See if original is in a section that can be discarded if the main
- symbol is not used.
+ symbol is not used. */
- Also consider case where we have resolution info and we know that
+ if (original-can_be_discarded_p ())
+original_discardable = true;
+ /* Also consider case where we have resolution info and we know that
original's definition is not going to be used. In this case we can not
create alias to original. */
- if (original-can_be_discarded_p ()
- || (node-resolution != LDPR_UNKNOWN
- !decl_binds_to_current_def_p (node-decl)))
-original_discardable = true;
+ if (node-resolution != LDPR_UNKNOWN
+ !decl_binds_to_current_def_p (node-decl))
+original_discardable = original_discarded = true;
/* Creating a symtab alias is the optimal way to merge.
It however can not be used in the following cases:
@@ -764,6 +766,7 @@ sem_function::merge (sem_item *alias_ite
(!DECL_COMDAT_GROUP (alias-decl)
|| (DECL_COMDAT_GROUP (alias-decl)
!= DECL_COMDAT_GROUP (original-decl
+ || original_discarded
|| !sem_item::target_supports_symbol_aliases_p ()
|| DECL_COMDAT_GROUP (alias-decl) != DECL_COMDAT_GROUP (original-decl))
{
@@ -773,7 +776,7 @@ sem_function::merge (sem_item *alias_ite
comdat group. Other compiler producing the body of the
another comdat group may make opossite decision and with unfortunate
linker choices this may close a loop. */
- if (DECL_COMDAT_GROUP (alias-decl)
+ if (DECL_COMDAT_GROUP (original-decl) DECL_COMDAT_GROUP (alias-decl)
(DECL_COMDAT_GROUP (alias-decl)
!= DECL_COMDAT_GROUP (original-decl)))
{
@@ -830,26 +833,27 @@ sem_function::merge (sem_item *alias_ite
/* Work out the symbol the wrapper should call.
If ORIGINAL is interposable, we need to call a local alias.
-Also produce local alias (if possible) as an optimization. */
- if (!original_discardable
- || (DECL_COMDAT_GROUP (original-decl)
- (DECL_COMDAT_GROUP (original-decl)
- == DECL_COMDAT_GROUP (alias-decl
+Also produce local alias (if possible) as an optimization.
+
+Local aliases can not be created inside comdat groups because that
+prevents inlining. */
+ if (!original_discardable !original-get_comdat_group ())
{
local_original
= dyn_cast cgraph_node * (original-noninterposable_alias ());
if (!local_original
original-get_availability () AVAIL_INTERPOSABLE)
local_original = original;
- /* If original is COMDAT local, we can not really redirect external
-callers to it. */
- if (original-comdat_local_p ())
- redirect_callers = false;
}
/* If we can not use local alias, fallback to the original
when possible. */
else if (original-get_availability () AVAIL_INTERPOSABLE)
local_original = original;
+
+ /* If original is COMDAT local, we can not really redirect calls outside
+of its comdat group to it. */
+ if (original-comdat_local_p ())
+redirect_callers = false;
if (!local_original)
{
if (dump_file)
@@ -1510,11 +1514,16 @@ sem_variable::merge (sem_item *alias_ite
adress of original and alias may be compared.\n\n);
return false;
}
+ if (DECL_COMDAT_GROUP (original-decl) != DECL_COMDAT_GROUP (alias-decl))
+{
+ if (dump_file)
+ fprintf (dump_file, Not unifying; alias cannot be created;
+across comdat group boundary\n\n);
+
+ return false;
+}
- if (original_discardable
- (!DECL_COMDAT_GROUP (original-decl)
-
Re: [RFC 1/2] Turn RETURN_ADDR_IN_PREVIOUS_FRAME into C expression
Richard, David, Eric,
could you please take a look and possibly approve the below changes for
sparc?
On Sat, Feb 28, 2015 at 9:14 PM, Max Filippov jcmvb...@gmail.com wrote:
This allows a target to support both windowed and non-windowed ABI.
2015-02-28 Max Filippov jcmvb...@gmail.com
gcc/
* builtins.c (expand_builtin_return_addr): Add
RETURN_ADDR_IN_PREVIOUS_FRAME to 'if' condition.
* config/sparc/sparc.h (RETURN_ADDR_IN_PREVIOUS_FRAME): Change
definition to 1.
* config/xtensa/xtensa.h (RETURN_ADDR_IN_PREVIOUS_FRAME):
Likewise.
---
gcc/builtins.c | 2 +-
gcc/config/sparc/sparc.h | 2 +-
gcc/config/xtensa/xtensa.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/gcc/builtins.c b/gcc/builtins.c
index fb871e6..0cc0c68 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -803,7 +803,7 @@ expand_builtin_return_addr (enum built_in_function
fndecl_code, int count)
pointer, but it can be accessed off the previous frame pointer by
reading the value from the register window save area. */
#ifdef RETURN_ADDR_IN_PREVIOUS_FRAME
- if (fndecl_code == BUILT_IN_RETURN_ADDRESS)
+ if (RETURN_ADDR_IN_PREVIOUS_FRAME fndecl_code ==
BUILT_IN_RETURN_ADDRESS)
count--;
#endif
diff --git a/gcc/config/sparc/sparc.h b/gcc/config/sparc/sparc.h
index c6100a1..27ad748 100644
--- a/gcc/config/sparc/sparc.h
+++ b/gcc/config/sparc/sparc.h
@@ -1293,7 +1293,7 @@ do {
\
access it from the current frame pointer. We can access it from the
previous frame pointer though by reading a value from the register window
save area. */
-#define RETURN_ADDR_IN_PREVIOUS_FRAME
+#define RETURN_ADDR_IN_PREVIOUS_FRAME 1
/* This is the offset of the return address to the true next instruction to
be
executed for the current function. */
diff --git a/gcc/config/xtensa/xtensa.h b/gcc/config/xtensa/xtensa.h
index 74ca240..14fe4bb 100644
--- a/gcc/config/xtensa/xtensa.h
+++ b/gcc/config/xtensa/xtensa.h
@@ -615,7 +615,7 @@ typedef struct xtensa_args
/* Define this if the return address of a particular stack frame is
accessed from the frame pointer of the previous stack frame. */
-#define RETURN_ADDR_IN_PREVIOUS_FRAME
+#define RETURN_ADDR_IN_PREVIOUS_FRAME 1
/* A C expression whose value is RTL representing the value of the
return address for the frame COUNT steps up from the current
--
1.8.1.4
--
Thanks.
-- Max
Re: [RFC 1/2] Turn RETURN_ADDR_IN_PREVIOUS_FRAME into C expression
On Sun, Mar 1, 2015 at 8:34 AM, augustine.sterl...@gmail.com augustine.sterl...@gmail.com wrote: On Sat, Feb 28, 2015 at 10:14 AM, Max Filippov jcmvb...@gmail.com wrote: This allows a target to support both windowed and non-windowed ABI. 2015-02-28 Max Filippov jcmvb...@gmail.com gcc/ * builtins.c (expand_builtin_return_addr): Add RETURN_ADDR_IN_PREVIOUS_FRAME to 'if' condition. * config/sparc/sparc.h (RETURN_ADDR_IN_PREVIOUS_FRAME): Change definition to 1. * config/xtensa/xtensa.h (RETURN_ADDR_IN_PREVIOUS_FRAME): Likewise. Unfortunately, I can't approve a change to spark.h, even though this seems quite innocuous to me. I'm not sure why it is even relevant to xtensa. Ok, I'll cc sparc maintainers. In the 2/2 RETURN_ADDR_IN_PREVIOUS_FRAME is changed for xtensa depending on the selected ABI. Without that, approved. -- Thanks. -- Max
