[patch] revert-empty-class-copy-avoidance-gcc12.patch

revert-empty-class-copy-avoidance-gcc12.patch (text/plain), 4.11 KB, created by Ionen Wolkens on 2022-07-04 13:02:46 UTC

This reverts upstream commit:

  94e2418780f1d13235f3e2e6e5c09dbe821c1ce3 c++: Avoid unnecessary empty class copy [94175].

which for reasons currently not understood seems to lead to various determinstic
and nondeterministic stack corruption modalities during gimplification on various
"Zen"-based AMD hosts, especially when building -march=znver1 targets with
likewise -march=znver1 -optimized gcc builds.

For gory details (mixed in with a lot of probably supersititious conclusions, ie:

  "GOT IT GUYS!!!! Doing a handstand and singing the Italian national anthem
  during the compile fixes it."

    -- not a real example,

see Gentoo bug #724314, and its several duplicates).

Somehow, adding these purportedly superflous copy steps back into the gimplification
process seems to fix some or all (so far it's 1 for 1!) affected systems.

Hopefully the patch is safe, but causes gcc to generate somewhat less efficient
code.  This is what the commitmsg implies.  But, be advised that I don't personally
understand the patch.  I only found it myself through git bisect drudgery ... so,
sorry, but I literally don't know what this patch will do to your computer!  It may
help, or it may well cause some kind of disaster or just do nothing...  but here's
hoping it works for you as well as I think it did for me! :S

-gmt
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -398,10 +398,6 @@
 	if (is_empty_class (TREE_TYPE (arg))
 	    && simple_empty_class_p (TREE_TYPE (arg), arg, INIT_EXPR))
 	  {
-	    while (TREE_CODE (arg) == TARGET_EXPR)
-	      /* We're disconnecting the initializer from its target,
-		 don't create a temporary.  */
-	      arg = TARGET_EXPR_INITIAL (arg);
 	    tree t = build0 (EMPTY_CLASS_EXPR, TREE_TYPE (arg));
 	    arg = build2 (COMPOUND_EXPR, TREE_TYPE (t), arg, t);
 	    CALL_EXPR_ARG (function, i) = arg;
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -339,10 +339,6 @@
 {
   if (TREE_CODE (op) == COMPOUND_EXPR)
     return simple_empty_class_p (type, TREE_OPERAND (op, 1), code);
-  if (SIMPLE_TARGET_EXPR_P (op)
-      && TYPE_HAS_TRIVIAL_DESTRUCTOR (type))
-    /* The TARGET_EXPR is itself a simple copy, look through it.  */
-    return simple_empty_class_p (type, TARGET_EXPR_INITIAL (op), code);
 
   if (TREE_CODE (op) == PARM_DECL
       && TREE_ADDRESSABLE (TREE_TYPE (op)))
@@ -526,11 +522,6 @@
 
 	else if (simple_empty_class_p (TREE_TYPE (op0), op1, code))
 	  {
-	    while (TREE_CODE (op1) == TARGET_EXPR)
-	      /* We're disconnecting the initializer from its target,
-		 don't create a temporary.  */
-	      op1 = TARGET_EXPR_INITIAL (op1);
-
 	    /* Remove any copies of empty classes.  Also drop volatile
 	       variables on the RHS to avoid infinite recursion from
 	       gimplify_expr trying to load the value.  */
@@ -545,9 +536,6 @@
 	    gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, post_p,
 			   is_gimple_lvalue, fb_lvalue);
 	    *expr_p = TREE_OPERAND (*expr_p, 0);
-	    if (code == RETURN_EXPR && REFERENCE_CLASS_P (*expr_p))
-	      /* Avoid 'return *<retval>;'  */
-	      *expr_p = TREE_OPERAND (*expr_p, 0);
 	  }
 	/* P0145 says that the RHS is sequenced before the LHS.
 	   gimplify_modify_expr gimplifies the RHS before the LHS, but that
@@ -762,6 +750,7 @@
 	      || TREE_CODE (TREE_OPERAND (*expr_p, 0)) == MODIFY_EXPR))
 	{
 	  expr_p = &TREE_OPERAND (*expr_p, 0);
+	  code = TREE_CODE (*expr_p);
 	  /* Avoid going through the INIT_EXPR case, which can
 	     degrade INIT_EXPRs into AGGR_INIT_EXPRs.  */
 	  goto modify_expr_case;
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5384,7 +5384,6 @@
    the initializer has void type, it's doing something more complicated.  */
 #define SIMPLE_TARGET_EXPR_P(NODE)				\
   (TREE_CODE (NODE) == TARGET_EXPR				\
-   && TARGET_EXPR_INITIAL (NODE)				\
    && !VOID_TYPE_P (TREE_TYPE (TARGET_EXPR_INITIAL (NODE))))
 
 /* True if T is a TARGET_EXPR for which we'll need to replace_decl to use it as
--- a/gcc/testsuite/g++.dg/abi/empty30.C
+++ b/gcc/testsuite/g++.dg/abi/empty30.C
@@ -1,14 +0,0 @@
-// PR c++/94175
-// { dg-do link }
-
-struct A {};
-extern A a;
-
-int i;
-__attribute ((noinline, noclone))
-void f(A) { ++i; }
-
-int main()
-{
-  f(a);
-}