On Sat, 24 Nov 2018 at 11:43, Johannes Pfau <johannesp...@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 <johannesp...@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 <johannesp...@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.