Skip to content

Commit

Permalink
d: Suboptimal codegen for __builtin_expect(cond, false)
Browse files Browse the repository at this point in the history
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 ab98db1)
  • Loading branch information
ibuclaw committed Jun 26, 2023
1 parent 016047f commit 0f54a73
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 12 deletions.
31 changes: 19 additions & 12 deletions gcc/d/d-convert.cc
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,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
Expand All @@ -635,20 +635,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);
}

Expand Down Expand Up @@ -845,7 +852,7 @@ convert_for_condition (tree expr, Type *type)
break;

default:
result = expr;
result = convert_for_rvalue (expr, type, type);
break;
}

Expand Down
22 changes: 22 additions & 0 deletions gcc/testsuite/gdc.dg/pr110359.d
Original file line number Diff line number Diff line change
@@ -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" } }

0 comments on commit 0f54a73

Please sign in to comment.