diff mbox

varpool/constpool bug

Message ID 567AE728.7020003@acm.org
State New
Headers show

Commit Message

Nathan Sidwell Dec. 23, 2015, 6:25 p.m. UTC 
On 12/23/15 09:48, Nathan Sidwell wrote:
> On 12/21/15 14:45, Jeff Law wrote:
>
>> With some kind of comment in decl_in_symtab_p indicating why we need to check
>> and filter on !DECL_IN_CONSTANT_POOL this is OK.
>
> Done, thanks.  (I half guessed HPPA might be such a port :)

sigh, there are  other routes by which constpool entries get into the varpool, 
and it appears other checks using symtab_in_varpool_p to figure out whether they 
need to do something.  Leading to powerpc having a problem. 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69033

powerpc inserts an array initializer constant pool entry into the varpool via 
tree_output_constant_def (varasm.c), that has an explicit call to:
  varpool_node::finalize_decl (decl);

so it seems intentional  that constpool objects can end up also in the varpool. 
  I notice that via this route varpool thinks the constant's value is available, 
  which is different to the behaviour when insertionwas via alias checking.

So, I think that either
1) alias checking shouldn't create varpool entries (i.e. use varpool::get rather 
than varpool::get_create). It wouldn't need to use the symtab_in_varpool_p 
predicate in that case.
2) Or varpool::get_create should take note of DECL_IN_CONSTANT_POOL and declare 
the value available
3) or build_constant_desc (varasm.c) should explicitly put the constant in the 
varpool, to match tree_output_constant_def?

I have reverted the cgraph.h change and am experimenting with option #1

nathan
diff mbox

Patch

2015-12-23  Nathan Sidwell  <nathan@acm.org>

	gcc/
	* cgraph.h (decl_in_symtab_p): Revert check DECL_IN_CONSTANT_POOL.

	gcc/testsuite/
	* gcc.dg/alias-15.c: Revert.

Index: cgraph.h
===================================================================
--- cgraph.h	(revision 231929)
+++ cgraph.h	(working copy)
@@ -2294,19 +2294,13 @@  symtab_node::real_symbol_p (void)
 }
 
 /* Return true if DECL should have entry in symbol table if used.
-   Those are functions and static & external non-constpool variables.
-   We do not expect constant pool variables in the varpool, as they're
-   not related to other variables, and simply lazily inserting them
-   using the regular interface results in varpool thinking they are
-   externally provided -- which results in erroneous assembly emission
-   as an undefined decl.  */
+   Those are functions and static & external veriables*/
 
 static inline bool
 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 231929)
+++ testsuite/gcc.dg/alias-15.c	(working copy)
@@ -1,15 +0,0 @@ 
-/* { 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" } } */