Re: Fix partitioning ICE with external comdats

[Andreas Schwab]

On Dez 17 2019, Jan Hubicka wrote:

> Index: symtab.c
> ===
> --- symtab.c  (revision 279178)
> +++ symtab.c  (working copy)
> @@ -1952,6 +1952,11 @@ symtab_node::get_partitioning_class (voi
>if (DECL_EXTERNAL (decl))
>  return SYMBOL_EXTERNAL;
>  
> +  /* Even static aliases of external functions as external.  Those can happen

s/as/are/

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Fix partitioning ICE with external comdats

[Jan Hubicka]

Hi,
while hacking firefox to work around ABI compatibility issues with LLVM
I ran into an ICE where comdat group was resolved externaly but contains
a static alias (for thunk). In this case parittioner attempts to put
that static definition into a partition which triggers an ICE.

Bootstrapped/regtested x86_64-linux, comitted.

* symtab.c (symtab_node::get_partitioning_class): Aliases of external
symbols are external.
Index: symtab.c
===
--- symtab.c(revision 279178)
+++ symtab.c(working copy)
@@ -1952,6 +1952,11 @@ symtab_node::get_partitioning_class (voi
   if (DECL_EXTERNAL (decl))
 return SYMBOL_EXTERNAL;
 
+  /* Even static aliases of external functions as external.  Those can happen
+ when COMDAT got resolved to non-IL implementation.  */
+  if (alias && DECL_EXTERNAL (ultimate_alias_target ()->decl))
+return SYMBOL_EXTERNAL;
+
   if (varpool_node *vnode = dyn_cast  (this))
 {
   if (alias && definition && !ultimate_alias_target ()->definition)

Fix partitioning ICE

[Jan Hubicka]

Hi,
this patch silences sanity check that makes sure that everything is in
partition 0 and thus boundary cost is 0 when there is only one partition.
In the testcase there is external initializer which does not get partitioned
and thus we end up with cost 1.  Fixed by not accounting references from
external initializers.

Bootstrapped/regtsted x86_64-linux, will commit it after finishing ltobootstrap.

Honza

* torture/pr85583.C: New testcase.

* lto-partition.c (account_reference_p): Do not account
references from aliases; do not account refernces from
external initializers.
Index: testsuite/g++.dg/torture/pr85583.C
===
--- testsuite/g++.dg/torture/pr85583.C  (nonexistent)
+++ testsuite/g++.dg/torture/pr85583.C  (working copy)
@@ -0,0 +1,13 @@
+/* { dg-do link } */
+class b {
+public:
+  virtual ~b();
+};
+template  class c : b {};
+class B {
+  c d;
+};
+extern template class c;
+int
+main(void) { B a; return 0; }
+
Index: lto/lto-partition.c
===
--- lto/lto-partition.c (revision 260258)
+++ lto/lto-partition.c (working copy)
@@ -439,13 +439,28 @@
 {
   if (cgraph_node *cnode = dyn_cast  (n1))
 n1 = cnode;
+  /* Do not account references from aliases - they are never split across
+ partitions.  */
+  if (n1->alias)
+return false;
   /* Do not account recursion - the code below will handle it incorrectly
- otherwise.  Also do not account references to external symbols.
- They will never become local.  */
+ otherwise.  Do not account references to external symbols: they will
+ never become local.  Finally do not account references to duplicated
+ symbols: they will be always local.  */
   if (n1 == n2 
-  || DECL_EXTERNAL (n2->decl)
-  || !n2->definition)
+  || !n2->definition
+  || n2->get_partitioning_class () != SYMBOL_PARTITION)
 return false;
+  /* If referring node is external symbol do not account it to boundary
+ cost. Those are added into units only to enable possible constant
+ folding and devirtulization.
+
+ Here we do not know if it will ever be added to some partition
+ (this is decided by compute_ltrans_boundary) and second it is not
+ that likely that constant folding will actually use the reference.  */
+  if (contained_in_symbol (n1)
+   ->get_partitioning_class () == SYMBOL_EXTERNAL)
+return false;
   return true;
 }