[PATCH, d] Fix IdentityExp comparison for complex floats

[Iain Buclaw]

On Sat, 24 Nov 2018 at 11:43, Johannes Pfau <johannespfau@gmail.com> wrote:
>
> Currently all identity comparisons for complex types (c1 is c2) return
> true. This is caused by the rrent implementation being only correct for
> real types, so this adds new code for the complex type cases.
>
> Fixes https://bugzilla.gdcproject.org/show_bug.cgi?id=309
>
> Tested on x86_64-linux-gnu and on the GDC CI
> (https://github.com/D-Programming-GDC/GDC/pull/768)
> --
> Johannes
>
>
> ---
> gcc/d/ChangeLog:
>
> 2018-11-24  Johannes Pfau  <johannespfau@gmail.com>
>
>         * expr.cc (ExprVisitor::visit(IdentityExp)): Add support for complex types.
>         (ExprVisitor::build_float_identity): New function.
>
> gcc/testsuite/ChangeLog:
>
> 2018-11-24  Johannes Pfau  <johannespfau@gmail.com>
>
>         * gdc.dg/runnable.d: Test IdentityExp for complex types.
>
>
>  gcc/d/expr.cc                   | 44 +++++++++++++++++++++++++--------
>  gcc/testsuite/gdc.dg/runnable.d | 22 +++++++++++++++++
>  2 files changed, 56 insertions(+), 10 deletions(-)
>
> diff --git a/gcc/d/expr.cc b/gcc/d/expr.cc
> index 9a1aad42d..7eef00ac0 100644
> --- a/gcc/d/expr.cc
> +++ b/gcc/d/expr.cc
> @@ -255,6 +255,21 @@ public:
>      this->result_ = build_condition (build_ctype (e->type), cond, t1, t2);
>    }
>
> +  /* Helper function for floating point identity comparison. Compare
> +     only well-defined bits, ignore padding (e.g. for X86 80bit real).  */
> +
> +  static tree build_float_identity (tree_code code, tree t1, tree t2)

It may be better to make this a static free function instead because...

[-- snip --]

> +       tree e1 = d_save_expr (build_expr (e->e1));
> +       tree e2 = d_save_expr (build_expr (e->e2));
> +       tree re1 = real_part (e1);
> +       tree im1 = imaginary_part (e1);
> +       tree re2 = real_part (e2);
> +       tree im2 = imaginary_part (e2);
>
> -       tree tmemcmp = builtin_decl_explicit (BUILT_IN_MEMCMP);
> -       tree size = size_int (TYPE_PRECISION (TREE_TYPE (t1)) / BITS_PER_UNIT);
> +       tree req = build_float_identity (code, re1, re2);
> +       tree ieq = build_float_identity (code, im1, im2);
>

... as I understand it, the preferred C++ coding convention requires
that static member function calls always be fully qualified.  So here,
it would be ExprVisitor::build_float_identity, as it makes it clear
where the function comes from.

Don't think its really required to put the result of real_part and
imaginary_part into a temporary, it's only used once, and there's
plenty of horizontal space to use.

Change itself looks OK and makes sense, we definitely don't want to do
memcmp() on the padding.

--
Iain.