A recent PTX patch of mine exposed a latent bug in varpool handling by exposing
more CSE opportunities.
In the attached testcase:
char *p;
void foo ()
{
p = "abc\n";
while (*p != '\n')
p++;
}
the RTL CSE pass checks whether SYMBOL_REF (p) and SYMBOL_REF ($LCO) are
related. LC0 is the constant pool entry for the string constant. In doing
this, we end up in compare_base_decls (alias.c), and decl_in_symtab_p thinks
'$LC0' should be in the symtab (it's a TREE_STATIC VAR_DECL). Consequently we
continue on to symtab_node::get_create (base2), and create a symtab entry for
$LCO. That entry thinks $LCO is defined in another TU. (this is the point at
which PTX blows up because $LCO is emitted to the assembly file as both a static
definition and an extern declaration.)
Either build_constant_desc (varasm.c) should put constants into the varpool or
decl_in_symtab_p should ignore constant pool VAR_DECLS. It appears to me that
the latter is the right choice, as constant pool entries can't be related to
regular varpool entries (right?)
Thus this patch augments the decl_in_symtab_p function to ignore constant pool
entries. While debugging I found the logic in compare_base_decls of 'in_symtab1
!= in_symtab2 || !in_symtab1' to be rather confusing, and simplified it to be
(the moral equivalent of) '!in_symtab1 || !in_symtab2'.
This looks target-agnostic, and manifests on (at least) x86_64-linux too --
except most targets don't need to emit declarations of undefined externs, so
they don't explode. Without the patch, the attached testcase shows an
additional varpool entry of:
*.LC0/2 (*.LC0) @0x7fa5c7d3a400
Type: variable
Visibility: asm_written artificial
References:
Referring:
Availability: not_available
Varpool flags: initialized read-only const-value-known
ok?
nathan
2015-12-19 Nathan Sidwell <nat...@acm.org>
gcc/
* alias.c (compare_base_decls): Simplify in-symtab check.
* cgraph.h (decl_in_symtab_p): Check DECL_IN_CONSTANT_POOL.
gcc/testsuite/
* gcc.dg/alias-15.c: New.
Index: alias.c
===================================================================
--- alias.c (revision 231841)
+++ alias.c (working copy)
@@ -2038,13 +2038,12 @@ compare_base_decls (tree base1, tree bas
if (base1 == base2)
return 1;
- bool in_symtab1 = decl_in_symtab_p (base1);
- bool in_symtab2 = decl_in_symtab_p (base2);
-
/* Declarations of non-automatic variables may have aliases. All other
decls are unique. */
- if (in_symtab1 != in_symtab2 || !in_symtab1)
+ if (!decl_in_symtab_p (base1)
+ || !decl_in_symtab_p (base2))
return 0;
+
ret = symtab_node::get_create (base1)->equal_address_to
(symtab_node::get_create (base2), true);
if (ret == 2)
Index: cgraph.h
===================================================================
--- cgraph.h (revision 231841)
+++ cgraph.h (working copy)
@@ -2301,6 +2301,7 @@ decl_in_symtab_p (const_tree decl)
{
return (TREE_CODE (decl) == FUNCTION_DECL
|| (TREE_CODE (decl) == VAR_DECL
+ && !DECL_IN_CONSTANT_POOL (decl)
&& (TREE_STATIC (decl) || DECL_EXTERNAL (decl))));
}
Index: testsuite/gcc.dg/alias-15.c
===================================================================
--- testsuite/gcc.dg/alias-15.c (revision 0)
+++ testsuite/gcc.dg/alias-15.c (working copy)
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-O2 -fdump-ipa-cgraph" } */
+
+/* RTL-level CSE shouldn't introduce LCO (for the string) into varpool */
+char *p;
+
+void foo ()
+{
+ p = "abc\n";
+
+ while (*p != '\n')
+ p++;
+}
+
+/* { dg-final { scan-ipa-dump-not "LC0" "cgraph" } } */
