diff mbox series

[committed] d: Suboptimal codegen for __builtin_expect(cond, false)

Message ID 20230626021222.419821-1-ibuclaw@gdcproject.org
State New
Headers show
Series [committed] d: Suboptimal codegen for __builtin_expect(cond, false) | expand

Commit Message

Iain Buclaw June 26, 2023, 2:12 a.m. UTC
Hi,

Since PR96435, both boolean objects and expressions have been evaluated
in the following way by the D front-end.

    (*(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.

Bootstrapped and regression tested on x86_64-linux-gnu/-m32, committed
to mainline, and backported to releases/gcc-13 through to gcc-10.

Regards,
Iain.

---
	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.
---
 gcc/d/d-convert.cc              | 31 +++++++++++++++++++------------
 gcc/testsuite/gdc.dg/pr110359.d | 22 ++++++++++++++++++++++
 2 files changed, 41 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/gdc.dg/pr110359.d
diff mbox series

Patch

diff --git a/gcc/d/d-convert.cc b/gcc/d/d-convert.cc
index cdbd69cf012..2b9d8e78fb6 100644
--- a/gcc/d/d-convert.cc
+++ b/gcc/d/d-convert.cc
@@ -619,7 +619,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
@@ -634,20 +634,27 @@  convert_for_rvalue (tree expr, Type *etype, Type *totype)
     {
       /* 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);
     }
 
@@ -844,7 +851,7 @@  convert_for_condition (tree expr, Type *type)
       break;
 
     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
index 00000000000..bf69201d9a5
--- /dev/null
+++ b/gcc/testsuite/gdc.dg/pr110359.d
@@ -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" } }