]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
d: Suboptimal codegen for __builtin_expect(cond, false)
authorIain Buclaw <ibuclaw@gdcproject.org>
Mon, 26 Jun 2023 01:24:27 +0000 (03:24 +0200)
committerIain Buclaw <ibuclaw@gdcproject.org>
Mon, 26 Jun 2023 01:33:05 +0000 (03:33 +0200)
Since PR96435, both boolean objects and expressions have been evaluated
in the following way.

    (*(ubyte*)&obj_or_expr) & 1

It has been noted that sometimes this can cause the back-end to optimize
in non-obvious ways - in particular with __builtin_expect.

This @safe feature is now restricted to just when reading the value of a
bool field that comes from a union.

PR d/110359

gcc/d/ChangeLog:

* d-convert.cc (convert_for_rvalue): Only apply the @safe boolean
conversion to boolean fields of a union.
(convert_for_condition): Call convert_for_rvalue in the default case.

gcc/testsuite/ChangeLog:

* gdc.dg/pr110359.d: New test.

(cherry picked from commit ab98db1e8c1b997414539f41b7fb814019497d8d)

gcc/d/d-convert.cc
gcc/testsuite/gdc.dg/pr110359.d [new file with mode: 0644]

index 2cfc2c8cc19b9def7280703736e1a8c46f1dea77..04923e1a29ce32dfd880e29e663e08bfb5893614 100644 (file)
@@ -609,7 +609,7 @@ convert_expr (tree exp, Type *etype, Type *totype)
   return result ? result : convert (build_ctype (totype), exp);
 }
 
-/* Return a TREE represenwation of EXPR, whose type has been converted from
+/* Return a TREE representation of EXPR, whose type has been converted from
  * ETYPE to TOTYPE, and is being used in an rvalue context.  */
 
 tree
@@ -625,20 +625,27 @@ convert_for_rvalue (tree expr, Type *etype, Type *totype)
     case Tbool:
       /* If casting from bool, the result is either 0 or 1, any other value
         violates @safe code, so enforce that it is never invalid.  */
-      if (CONSTANT_CLASS_P (expr))
-       result = d_truthvalue_conversion (expr);
-      else
+      for (tree ref = expr; TREE_CODE (ref) == COMPONENT_REF;
+          ref = TREE_OPERAND (ref, 0))
        {
-         /* Reinterpret the boolean as an integer and test the first bit.
-            The generated code should end up being equivalent to:
+         /* If the expression is a field that's part of a union, reinterpret
+            the boolean as an integer and test the first bit.  The generated
+            code should end up being equivalent to:
                *cast(ubyte *)&expr & 1;  */
-         machine_mode bool_mode = TYPE_MODE (TREE_TYPE (expr));
-         tree mtype = lang_hooks.types.type_for_mode (bool_mode, 1);
-         result = fold_build2 (BIT_AND_EXPR, mtype,
-                               build_vconvert (mtype, expr),
-                               build_one_cst (mtype));
+         if (TREE_CODE (TREE_TYPE (TREE_OPERAND (ref, 0))) == UNION_TYPE)
+           {
+             machine_mode bool_mode = TYPE_MODE (TREE_TYPE (expr));
+             tree mtype = lang_hooks.types.type_for_mode (bool_mode, 1);
+             result = fold_build2 (BIT_AND_EXPR, mtype,
+                                   build_vconvert (mtype, expr),
+                                   build_one_cst (mtype));
+             break;
+           }
        }
 
+      if (result == NULL_TREE)
+       result = d_truthvalue_conversion (expr);
+
       result = convert (build_ctype (tbtype), result);
       break;
     }
@@ -826,7 +833,7 @@ convert_for_condition (tree expr, Type *type)
       }
 
     default:
-      result = expr;
+      result = convert_for_rvalue (expr, type, type);
       break;
     }
 
diff --git a/gcc/testsuite/gdc.dg/pr110359.d b/gcc/testsuite/gdc.dg/pr110359.d
new file mode 100644 (file)
index 0000000..bf69201
--- /dev/null
@@ -0,0 +1,22 @@
+// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110359
+// { dg-do compile }
+// { dg-options "-fdump-tree-original" }
+double pow(in double x, in ulong p)
+{
+    import gcc.builtins : __builtin_expect;
+    if (__builtin_expect(p == 0, false))
+        return 1;
+    if (__builtin_expect(p == 1, false))
+        return x;
+
+    double s = x;
+    double v = 1;
+    for (ulong i = p; i > 1; i >>= 1)
+    {
+        v = (i & 0x1) ? s * v : v;
+        s = s * s;
+    }
+    return v * s;
+}
+// { dg-final { scan-tree-dump "if \\(__builtin_expect \\(p == 0, 0\\) != 0\\)" "original" } }
+// { dg-final { scan-tree-dump "if \\(__builtin_expect \\(p == 1, 0\\) != 0\\)" "original" } }