[PATCH] Fix PR50494

Richard Biener rguenther@suse.de
Wed Feb 13 12:04:00 GMT 2013


This should fix PR50494 - when we gimplify a local initializer
via a block copy and emit a constant to the constant pool we
stream the representative VAR_DECL with LTO, including its
eventually changed alignment.  When we then lookup RTL for this
constant at

  /* If this variable belongs to the global constant pool, retrieve the
     pre-computed RTL or recompute it in LTO mode.  */
  if (TREE_CODE (decl) == VAR_DECL && DECL_IN_CONSTANT_POOL (decl))
    {
      SET_DECL_RTL (decl, output_constant_def (DECL_INITIAL (decl), 1));
      return;

we create yet another VAR_DECL for the constant representation.
But we ignore the alignment of the originally used representative
and instead use default alignment.  This breaks on powerpc
where the vectorizer increases alignment of such a representative
VAR_DECL coming from LTO (and thus not yet registered with the
constant output machinery), as the generated constant will not
have that increased alignment.

The fix should be to never create a new VAR_DECL for the
constant representation in the above path but re-use 'decl'
which has whatever alignment it has.

LTO bootstrap and regtest on x86_64-unknown-linux-gnu running
(not sure if this will excercise this a lot), I'm waiting for
ppc folks to confirm the bug is gone with the patch.

Eric, does this look correct?

Thanks,
Richard.

2013-02-13  Richard Biener  <rguenther@suse.de>

	PR lto/50494
	* varasm.c (output_constant_def_1): Get the decl representing
	the constant as argument.
	(output_constant_def): Wrap output_constant_def_1.
	(make_decl_rtl): Use output_constant_def_1 with the decl
	representing the constant.
	(build_constant_desc): Optionally re-use a decl already
	representing the constant.
	(tree_output_constant_def): Adjust.

Index: gcc/varasm.c
===================================================================
*** gcc/varasm.c	(revision 195997)
--- gcc/varasm.c	(working copy)
*************** static void asm_output_aligned_bss (FILE
*** 126,131 ****
--- 126,132 ----
  #endif /* BSS_SECTION_ASM_OP */
  static void mark_weak (tree);
  static void output_constant_pool (const char *, tree);
+ static rtx output_constant_def_1 (tree, tree, int);
  
  /* Well-known sections, each one associated with some sort of *_ASM_OP.  */
  section *text_section;
*************** make_decl_rtl (tree decl)
*** 1186,1192 ****
       pre-computed RTL or recompute it in LTO mode.  */
    if (TREE_CODE (decl) == VAR_DECL && DECL_IN_CONSTANT_POOL (decl))
      {
!       SET_DECL_RTL (decl, output_constant_def (DECL_INITIAL (decl), 1));
        return;
      }
  
--- 1187,1194 ----
       pre-computed RTL or recompute it in LTO mode.  */
    if (TREE_CODE (decl) == VAR_DECL && DECL_IN_CONSTANT_POOL (decl))
      {
!       SET_DECL_RTL (decl, output_constant_def_1 (DECL_INITIAL (decl),
! 						 decl, 1));
        return;
      }
  
*************** get_constant_size (tree exp)
*** 3073,3088 ****
     Make a constant descriptor to enter EXP in the hash table.
     Assign the label number and construct RTL to refer to the
     constant's location in memory.
     Caller is responsible for updating the hash table.  */
  
  static struct constant_descriptor_tree *
! build_constant_desc (tree exp)
  {
    struct constant_descriptor_tree *desc;
    rtx symbol, rtl;
    char label[256];
    int labelno;
-   tree decl;
  
    desc = ggc_alloc_constant_descriptor_tree ();
    desc->value = copy_constant (exp);
--- 3075,3090 ----
     Make a constant descriptor to enter EXP in the hash table.
     Assign the label number and construct RTL to refer to the
     constant's location in memory.
+    If DECL is non-NULL use it as VAR_DECL associated with the constant.
     Caller is responsible for updating the hash table.  */
  
  static struct constant_descriptor_tree *
! build_constant_desc (tree exp, tree decl)
  {
    struct constant_descriptor_tree *desc;
    rtx symbol, rtl;
    char label[256];
    int labelno;
  
    desc = ggc_alloc_constant_descriptor_tree ();
    desc->value = copy_constant (exp);
*************** build_constant_desc (tree exp)
*** 3096,3123 ****
    ASM_GENERATE_INTERNAL_LABEL (label, "LC", labelno);
  
    /* Construct the VAR_DECL associated with the constant.  */
!   decl = build_decl (UNKNOWN_LOCATION, VAR_DECL, get_identifier (label),
! 		     TREE_TYPE (exp));
!   DECL_ARTIFICIAL (decl) = 1;
!   DECL_IGNORED_P (decl) = 1;
!   TREE_READONLY (decl) = 1;
!   TREE_STATIC (decl) = 1;
!   TREE_ADDRESSABLE (decl) = 1;
!   /* We don't set the RTL yet as this would cause varpool to assume that the
!      variable is referenced.  Moreover, it would just be dropped in LTO mode.
!      Instead we set the flag that will be recognized in make_decl_rtl.  */
!   DECL_IN_CONSTANT_POOL (decl) = 1;
!   DECL_INITIAL (decl) = desc->value;
!   /* ??? CONSTANT_ALIGNMENT hasn't been updated for vector types on most
!      architectures so use DATA_ALIGNMENT as well, except for strings.  */
!   if (TREE_CODE (exp) == STRING_CST)
      {
  #ifdef CONSTANT_ALIGNMENT
!       DECL_ALIGN (decl) = CONSTANT_ALIGNMENT (exp, DECL_ALIGN (decl));
  #endif
      }
-   else
-     align_variable (decl, 0);
  
    /* Now construct the SYMBOL_REF and the MEM.  */
    if (use_object_blocks_p ())
--- 3098,3129 ----
    ASM_GENERATE_INTERNAL_LABEL (label, "LC", labelno);
  
    /* Construct the VAR_DECL associated with the constant.  */
!   if (decl == NULL_TREE)
      {
+       decl = build_decl (UNKNOWN_LOCATION, VAR_DECL, get_identifier (label),
+ 			 TREE_TYPE (exp));
+       DECL_ARTIFICIAL (decl) = 1;
+       DECL_IGNORED_P (decl) = 1;
+       TREE_READONLY (decl) = 1;
+       TREE_STATIC (decl) = 1;
+       TREE_ADDRESSABLE (decl) = 1;
+       /* We don't set the RTL yet as this would cause varpool to assume that
+ 	 the variable is referenced.  Moreover, it would just be dropped in
+ 	 LTO mode.  Instead we set the flag that will be recognized in
+ 	 make_decl_rtl.  */
+       DECL_IN_CONSTANT_POOL (decl) = 1;
+       DECL_INITIAL (decl) = desc->value;
+       /* ??? CONSTANT_ALIGNMENT hasn't been updated for vector types on most
+ 	 architectures so use DATA_ALIGNMENT as well, except for strings.  */
+       if (TREE_CODE (exp) == STRING_CST)
+ 	{
  #ifdef CONSTANT_ALIGNMENT
! 	  DECL_ALIGN (decl) = CONSTANT_ALIGNMENT (exp, DECL_ALIGN (decl));
  #endif
+ 	}
+       else
+ 	align_variable (decl, 0);
      }
  
    /* Now construct the SYMBOL_REF and the MEM.  */
    if (use_object_blocks_p ())
*************** build_constant_desc (tree exp)
*** 3154,3160 ****
  }
  
  /* Return an rtx representing a reference to constant data in memory
!    for the constant expression EXP.
  
     If assembler code for such a constant has already been output,
     return an rtx to refer to it.
--- 3160,3166 ----
  }
  
  /* Return an rtx representing a reference to constant data in memory
!    for the constant expression EXP with the associated DECL.
  
     If assembler code for such a constant has already been output,
     return an rtx to refer to it.
*************** build_constant_desc (tree exp)
*** 3166,3173 ****
  
     `const_desc_table' records which constants already have label strings.  */
  
! rtx
! output_constant_def (tree exp, int defer)
  {
    struct constant_descriptor_tree *desc;
    struct constant_descriptor_tree key;
--- 3172,3179 ----
  
     `const_desc_table' records which constants already have label strings.  */
  
! static rtx
! output_constant_def_1 (tree exp, tree decl, int defer)
  {
    struct constant_descriptor_tree *desc;
    struct constant_descriptor_tree key;
*************** output_constant_def (tree exp, int defer
*** 3182,3188 ****
    desc = (struct constant_descriptor_tree *) *loc;
    if (desc == 0)
      {
!       desc = build_constant_desc (exp);
        desc->hash = key.hash;
        *loc = desc;
      }
--- 3188,3194 ----
    desc = (struct constant_descriptor_tree *) *loc;
    if (desc == 0)
      {
!       desc = build_constant_desc (exp, decl);
        desc->hash = key.hash;
        *loc = desc;
      }
*************** output_constant_def (tree exp, int defer
*** 3191,3196 ****
--- 3197,3211 ----
    return desc->rtl;
  }
  
+ /* Like output_constant_def but create a new decl representing the
+    constant if necessary.  */
+ 
+ rtx
+ output_constant_def (tree exp, int defer)
+ {
+   return  output_constant_def_1  (exp, NULL_TREE, defer);
+ }
+ 
  /* Subroutine of output_constant_def: Decide whether or not we need to
     output the constant DESC now, and if so, do it.  */
  static void
*************** tree_output_constant_def (tree exp)
*** 3327,3333 ****
    desc = (struct constant_descriptor_tree *) *loc;
    if (desc == 0)
      {
!       desc = build_constant_desc (exp);
        desc->hash = key.hash;
        *loc = desc;
      }
--- 3342,3348 ----
    desc = (struct constant_descriptor_tree *) *loc;
    if (desc == 0)
      {
!       desc = build_constant_desc (exp, NULL_TREE);
        desc->hash = key.hash;
        *loc = desc;
      }



More information about the Gcc-patches mailing list