diff mbox series

Hash ODR name for OBJ_TYPE_REF

Message ID 20201118073034.GA46107@kam.mff.cuni.cz
State New
Headers show
Series Hash ODR name for OBJ_TYPE_REF | expand

Commit Message

Jan Hubicka Nov. 18, 2020, 7:30 a.m. UTC
Hi,
main purpose of obj_type_ref is to hold the type that was used in
virutal call.  We do not hash this info in hash_operand that causes a
lot of miscompares at ICF time.  With LTO this is quite important for
icf performance and in that case we do have manged type names (for
non-anonymous types)

Building libxul without patch we get 1477890 miscompares at:
libxul.so.wpa.076i.icf:  false returned: 'operand_equal_p failed' in compare_operand at ../../gcc/ipa-icf-gimple.c:356
With patch this turns into 242454.

Bootstrapped/regtested x86_64-linux, OK?
Honza
	PR ipa/92535
	* fold-const.c (operand_compare::hash_operand): Hash ODR name of
	obj_type_ref_class.

Comments

Richard Biener Nov. 18, 2020, 7:32 a.m. UTC | #1
On Wed, 18 Nov 2020, Jan Hubicka wrote:

> Hi,
> main purpose of obj_type_ref is to hold the type that was used in
> virutal call.  We do not hash this info in hash_operand that causes a
> lot of miscompares at ICF time.  With LTO this is quite important for
> icf performance and in that case we do have manged type names (for
> non-anonymous types)
> 
> Building libxul without patch we get 1477890 miscompares at:
> libxul.so.wpa.076i.icf:  false returned: 'operand_equal_p failed' in compare_operand at ../../gcc/ipa-icf-gimple.c:356
> With patch this turns into 242454.
> 
> Bootstrapped/regtested x86_64-linux, OK?

OK.

Richard.

> Honza
> 	PR ipa/92535
> 	* fold-const.c (operand_compare::hash_operand): Hash ODR name of
> 	obj_type_ref_class.
> diff --git a/gcc/fold-const.c b/gcc/fold-const.c
> index ddf18f27cb7..e759ddb1e60 100644
> --- a/gcc/fold-const.c
> +++ b/gcc/fold-const.c
> @@ -3866,6 +3866,16 @@ operand_compare::hash_operand (const_tree t, inchash::hash &hstate,
>  	      inchash::add_expr (OBJ_TYPE_REF_EXPR (t), hstate, flags);
>  	      inchash::add_expr (OBJ_TYPE_REF_TOKEN (t), hstate, flags);
>  	      inchash::add_expr (OBJ_TYPE_REF_OBJECT (t), hstate, flags);
> +	      if (tree c = obj_type_ref_class (t))
> +	      {
> +		c = TYPE_NAME (TYPE_MAIN_VARIANT (c));
> +		/* We compute mangled names only when free_lang_data is run.
> +		   In that case we can hash precisely.  */
> +		if (DECL_ASSEMBLER_NAME_SET_P (c))
> +		  hstate.add_object
> +			 (IDENTIFIER_HASH_VALUE
> +				 (DECL_ASSEMBLER_NAME (c)));
> +	      }
>  	      return;
>  	    default:
>  	      break;
>
Jan Hubicka Nov. 19, 2020, 12:48 p.m. UTC | #2
Hi,
this is variant of patch I comitted (with a mistake fixed by Richard, my
apologizes for it).  It turned out that the hunk handling OBJ_TYPE_REF
in operand_compare::hash_operand was on a wrong spot in operand_equal_p
that probably happened during mering the patch.

Also with Ada LTO bootstrap I noticed I need to check for TYPE_NAME to
be TYPE_DECL and not IDENTIFIER_POINTER which Ada does before free lang
data.

lto-bootstrapped/regtested x86_64-linux.

	* fold-const.c (operand_compare::operand_equal_p): More OBJ_TYPE_REF
	matching to correct place; drop OEP_ADDRESS_OF for TOKEN, OBJECT and
	class.
	(operand_compare::hash_operand): Hash ODR type for OBJ_TYPE_REF.
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index ddf18f27cb7..136f01b6b35 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -3346,24 +3350,6 @@ operand_compare::operand_equal_p (const_tree arg0, const_tree arg1,
 	  flags &= ~OEP_ADDRESS_OF;
 	  return OP_SAME (1) && OP_SAME (2);
 
-	/* Virtual table call.  */
-	case OBJ_TYPE_REF:
-	  {
-	    if (!operand_equal_p (OBJ_TYPE_REF_EXPR (arg0),
-				  OBJ_TYPE_REF_EXPR (arg1), flags))
-	      return false;
-	    if (tree_to_uhwi (OBJ_TYPE_REF_TOKEN (arg0))
-		!= tree_to_uhwi (OBJ_TYPE_REF_TOKEN (arg1)))
-	      return false;
-	    if (!operand_equal_p (OBJ_TYPE_REF_OBJECT (arg0),
-				  OBJ_TYPE_REF_OBJECT (arg1), flags))
-	      return false;
-	    if (!types_same_for_odr (obj_type_ref_class (arg0),
-				     obj_type_ref_class (arg1)))
-	      return false;
-	    return true;
-	  }
-
 	default:
 	  return false;
 	}
@@ -3442,6 +3428,23 @@ operand_compare::operand_equal_p (const_tree arg0, const_tree arg1,
 	    return OP_SAME (0);
 	  return false;
 
+	case OBJ_TYPE_REF:
+	/* Virtual table reference.  */
+	if (!operand_equal_p (OBJ_TYPE_REF_EXPR (arg0),
+			      OBJ_TYPE_REF_EXPR (arg1), flags))
+	  return false;
+	flags &= ~OEP_ADDRESS_OF;
+	if (tree_to_uhwi (OBJ_TYPE_REF_TOKEN (arg0))
+	    != tree_to_uhwi (OBJ_TYPE_REF_TOKEN (arg1)))
+	  return false;
+	if (!operand_equal_p (OBJ_TYPE_REF_OBJECT (arg0),
+			      OBJ_TYPE_REF_OBJECT (arg1), flags))
+	  return false;
+	if (!types_same_for_odr (obj_type_ref_class (arg0),
+				 obj_type_ref_class (arg1)))
+	  return false;
+	return true;
+
 	default:
 	  return false;
 	}
@@ -3861,11 +3864,23 @@ operand_compare::hash_operand (const_tree t, inchash::hash &hstate,
 	      hash_operand (TARGET_EXPR_SLOT (t), hstate, flags);
 	      return;
 
-	    /* Virtual table call.  */
 	    case OBJ_TYPE_REF:
+	    /* Virtual table reference.  */
 	      inchash::add_expr (OBJ_TYPE_REF_EXPR (t), hstate, flags);
+	      flags &= ~OEP_ADDRESS_OF;
 	      inchash::add_expr (OBJ_TYPE_REF_TOKEN (t), hstate, flags);
 	      inchash::add_expr (OBJ_TYPE_REF_OBJECT (t), hstate, flags);
+	      if (tree c = obj_type_ref_class (t))
+		{
+		  c = TYPE_NAME (TYPE_MAIN_VARIANT (c));
+		  /* We compute mangled names only when free_lang_data is run.
+		     In that case we can hash precisely.  */
+		  if (TREE_CODE (c) == TYPE_DECL
+		      && DECL_ASSEMBLER_NAME_SET_P (c))
+		    hstate.add_object
+			   (IDENTIFIER_HASH_VALUE
+				   (DECL_ASSEMBLER_NAME (c)));
+		}
 	      return;
 	    default:
 	      break;
diff mbox series

Patch

diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index ddf18f27cb7..e759ddb1e60 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -3866,6 +3866,16 @@  operand_compare::hash_operand (const_tree t, inchash::hash &hstate,
 	      inchash::add_expr (OBJ_TYPE_REF_EXPR (t), hstate, flags);
 	      inchash::add_expr (OBJ_TYPE_REF_TOKEN (t), hstate, flags);
 	      inchash::add_expr (OBJ_TYPE_REF_OBJECT (t), hstate, flags);
+	      if (tree c = obj_type_ref_class (t))
+	      {
+		c = TYPE_NAME (TYPE_MAIN_VARIANT (c));
+		/* We compute mangled names only when free_lang_data is run.
+		   In that case we can hash precisely.  */
+		if (DECL_ASSEMBLER_NAME_SET_P (c))
+		  hstate.add_object
+			 (IDENTIFIER_HASH_VALUE
+				 (DECL_ASSEMBLER_NAME (c)));
+	      }
 	      return;
 	    default:
 	      break;