Re: [PATCH, V2, d] Fix IdentityExp comparison for complex floats
On Mon, 17 Dec 2018 at 23:05, Iain Buclaw wrote:
>
> On Wed, 28 Nov 2018 at 23:46, Iain Buclaw wrote:
> >
> > On Wed, 28 Nov 2018 at 22:32, Johannes Pfau wrote:
> > >
> > > Next version, addresses the review comments.
> > >
> > > Tested at https://github.com/D-Programming-GDC/GDC/pull/768
> > > ---
> > > gcc/d/ChangeLog:
> > >
> > > 2018-11-28 Johannes Pfau
> > >
> > > * expr.cc (ExprVisitor::visit(IdentityExp)): Add support for
> > > complex types.
> > > (build_float_identity): New function.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > 2018-11-28 Johannes Pfau
> > >
> > > * gdc.dg/runnable.d: Test IdentityExp for complex types.
> > >
> > >
> > > gcc/d/expr.cc | 40 -
> > > gcc/testsuite/gdc.dg/runnable.d | 22 ++
> > > 2 files changed, 51 insertions(+), 11 deletions(-)
> > >
> >
> > As I've said before, looks reasonable to me. Thanks.
> >
>
> I'll send a supplementary patch, and commit both together.
>
Committed to trunk along with patch that handles creal fields.
Bootstrapped and regression tested on x86_64-linux-gnu.
--
Iain
---
gcc/d/ChangeLog:
2019-01-20 Iain Buclaw
* d-codegen.cc (identity_compare_p): Return false if seen built-in
type with padding.
(build_float_identity): Moved here from expr.cc.
(lower_struct_comparison): Handle real and complex types.
* d-tree.h (build_float_identity): New.
* expr.cc (build_float_identity): Move to d-codegen.cc.
gcc/testsuite/ChangeLog:
2019-01-20 Iain Buclaw
* gdc.dg/runnable.d: Add more tests for comparing complex types.
---
diff --git a/gcc/d/d-codegen.cc b/gcc/d/d-codegen.cc
index 7ca0acffcc4..58c8257c63c 100644
--- a/gcc/d/d-codegen.cc
+++ b/gcc/d/d-codegen.cc
@@ -798,15 +798,21 @@ identity_compare_p (StructDeclaration *sd)
for (size_t i = 0; i < sd->fields.dim; i++)
{
VarDeclaration *vd = sd->fields[i];
+ Type *tb = vd->type->toBasetype ();
/* Check inner data structures. */
- if (vd->type->ty == Tstruct)
+ if (tb->ty == Tstruct)
{
- TypeStruct *ts = (TypeStruct *) vd->type;
+ TypeStruct *ts = (TypeStruct *) tb;
if (!identity_compare_p (ts->sym))
return false;
}
+ /* Check for types that may have padding. */
+ if ((tb->ty == Tcomplex80 || tb->ty == Tfloat80 || tb->ty == Timaginary80)
+ && Target::realpad != 0)
+ return false;
+
if (offset <= vd->offset)
{
/* There's a hole in the struct. */
@@ -824,6 +830,20 @@ identity_compare_p (StructDeclaration *sd)
return true;
}
+/* Build a floating-point identity comparison between T1 and T2, ignoring any
+ excessive padding in the type. CODE is EQ_EXPR or NE_EXPR comparison. */
+
+tree
+build_float_identity (tree_code code, tree t1, tree t2)
+{
+ tree tmemcmp = builtin_decl_explicit (BUILT_IN_MEMCMP);
+ tree size = size_int (TYPE_PRECISION (TREE_TYPE (t1)) / BITS_PER_UNIT);
+
+ tree result = build_call_expr (tmemcmp, 3, build_address (t1),
+ build_address (t2), size);
+ return build_boolop (code, result, integer_zero_node);
+}
+
/* Lower a field-by-field equality expression between T1 and T2 of type SD.
CODE is the EQ_EXPR or NE_EXPR comparison. */
@@ -859,29 +879,45 @@ lower_struct_comparison (tree_code code, StructDeclaration *sd,
for (size_t i = 0; i < sd->fields.dim; i++)
{
VarDeclaration *vd = sd->fields[i];
+ Type *type = vd->type->toBasetype ();
tree sfield = get_symbol_decl (vd);
tree t1ref = component_ref (t1, sfield);
tree t2ref = component_ref (t2, sfield);
tree tcmp;
- if (vd->type->ty == Tstruct)
+ if (type->ty == Tstruct)
{
/* Compare inner data structures. */
- StructDeclaration *decl = ((TypeStruct *) vd->type)->sym;
+ StructDeclaration *decl = ((TypeStruct *) type)->sym;
tcmp = lower_struct_comparison (code, decl, t1ref, t2ref);
}
+ else if (type->ty != Tvector && type->isintegral ())
+ {
+ /* Integer comparison, no special handling required. */
+ tcmp = build_boolop (code, t1ref, t2ref);
+ }
+ else if (type->ty != Tvector && type->isfloating ())
+ {
+ /* Floating-point comparison, don't compare padding in type. */
+ if (!type->iscomplex ())
+ tcmp = build_float_identity (code, t1ref, t2ref);
+ else
+ {
+ tree req = build_float_identity (code, real_part (t1ref),
+ real_part (t2ref));
+ tree ieq = build_float_identity (code, imaginary_part (t1ref),
+ imaginary_part (t2ref));
+
+ tcmp = build_boolop (tcode, req, ieq);
+ }
+ }
else
{
- tree stype = build_ctype (vd->type);
+ tree stype = build_ctype (type);
opt_scalar_int_mode mode = int_mode_for_mode (TYPE_MODE (stype));
- if (vd->type->ty != Tvector && vd->type->isintegral ())
- {
- /* Integer comparison, no special handling required. */
- tcmp = build_boolop (code, t1ref, t2ref);
- }
- else if (mode.
Re: [PATCH, V2, d] Fix IdentityExp comparison for complex floats
On Wed, 28 Nov 2018 at 23:46, Iain Buclaw wrote:
>
> On Wed, 28 Nov 2018 at 22:32, Johannes Pfau wrote:
> >
> > Next version, addresses the review comments.
> >
> > Tested at https://github.com/D-Programming-GDC/GDC/pull/768
> > ---
> > gcc/d/ChangeLog:
> >
> > 2018-11-28 Johannes Pfau
> >
> > * expr.cc (ExprVisitor::visit(IdentityExp)): Add support for
> > complex types.
> > (build_float_identity): New function.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 2018-11-28 Johannes Pfau
> >
> > * gdc.dg/runnable.d: Test IdentityExp for complex types.
> >
> >
> > gcc/d/expr.cc | 40 -
> > gcc/testsuite/gdc.dg/runnable.d | 22 ++
> > 2 files changed, 51 insertions(+), 11 deletions(-)
> >
>
> As I've said before, looks reasonable to me. Thanks.
>
I noticed that this hasn't been pushed in yet, I was about to go ahead
and commit it, however... there's another case to consider, structs
with creal fields.
This test - modified from your patch - is another that should pass but
currently doesn't.
---
struct CReal
{
creal value;
}
CReal s1 = CReal(+0.0 + 0.0i);
CReal s2 = CReal(+0.0 - 0.0i);
CReal s3 = CReal(-0.0 + 0.0i);
CReal s4 = CReal(+0.0 + 0.0i);
assert(s1 !is s2);
assert(s1 !is s3);
assert(s2 !is s3);
assert(s1 is s4);
assert(!(s1 is s2));
assert(!(s1 is s3));
assert(!(s2 is s3));
assert(!(s1 !is s4));
---
I'll send a supplementary patch, and commit both together.
--
Iain
Re: [PATCH, V2, d] Fix IdentityExp comparison for complex floats
On Wed, 28 Nov 2018 at 22:32, Johannes Pfau wrote: > > Next version, addresses the review comments. > > Tested at https://github.com/D-Programming-GDC/GDC/pull/768 > --- > gcc/d/ChangeLog: > > 2018-11-28 Johannes Pfau > > * expr.cc (ExprVisitor::visit(IdentityExp)): Add support for complex > types. > (build_float_identity): New function. > > gcc/testsuite/ChangeLog: > > 2018-11-28 Johannes Pfau > > * gdc.dg/runnable.d: Test IdentityExp for complex types. > > > gcc/d/expr.cc | 40 - > gcc/testsuite/gdc.dg/runnable.d | 22 ++ > 2 files changed, 51 insertions(+), 11 deletions(-) > As I've said before, looks reasonable to me. Thanks. -- Iain
[PATCH, V2, d] Fix IdentityExp comparison for complex floats
Next version, addresses the review comments.
Tested at https://github.com/D-Programming-GDC/GDC/pull/768
---
gcc/d/ChangeLog:
2018-11-28 Johannes Pfau
* expr.cc (ExprVisitor::visit(IdentityExp)): Add support for complex
types.
(build_float_identity): New function.
gcc/testsuite/ChangeLog:
2018-11-28 Johannes Pfau
* gdc.dg/runnable.d: Test IdentityExp for complex types.
gcc/d/expr.cc | 40 -
gcc/testsuite/gdc.dg/runnable.d | 22 ++
2 files changed, 51 insertions(+), 11 deletions(-)
diff --git a/gcc/d/expr.cc b/gcc/d/expr.cc
index 9a1aad42ddc..91cb02f1e9a 100644
--- a/gcc/d/expr.cc
+++ b/gcc/d/expr.cc
@@ -42,6 +42,20 @@ along with GCC; see the file COPYING3. If not see
#include "d-tree.h"
+/* 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)
+{
+ /* For floating-point values, identity is defined as the bits in the
+ operands being identical. */
+ tree tmemcmp = builtin_decl_explicit (BUILT_IN_MEMCMP);
+ tree size = size_int (TYPE_PRECISION (TREE_TYPE (t1)) / BITS_PER_UNIT);
+
+ tree result = build_call_expr (tmemcmp, 3, build_address (t1),
+build_address (t2), size);
+ return build_boolop (code, result, integer_zero_node);
+}
/* Implements the visitor interface to build the GCC trees of all Expression
AST classes emitted from the D Front-end.
@@ -275,19 +289,23 @@ public:
this->result_ = d_convert (build_ctype (e->type),
build_boolop (code, t1, t2));
}
-else if (tb1->isfloating () && tb1->ty != Tvector)
+else if (tb1->iscomplex () && tb1->ty != Tvector)
{
- /* For floating-point values, identity is defined as the bits in the
- operands being identical. */
- tree t1 = d_save_expr (build_expr (e->e1));
- tree t2 = d_save_expr (build_expr (e->e2));
-
- tree tmemcmp = builtin_decl_explicit (BUILT_IN_MEMCMP);
- tree size = size_int (TYPE_PRECISION (TREE_TYPE (t1)) / BITS_PER_UNIT);
+ tree e1 = d_save_expr (build_expr (e->e1));
+ tree e2 = d_save_expr (build_expr (e->e2));
+ tree req = build_float_identity (code, real_part (e1), real_part (e2));
+ tree ieq = build_float_identity (code, imaginary_part (e1),
imaginary_part (e2));
- tree result = build_call_expr (tmemcmp, 3, build_address (t1),
- build_address (t2), size);
- this->result_ = build_boolop (code, result, integer_zero_node);
+ if (code == EQ_EXPR)
+ this->result_ = build_boolop (TRUTH_ANDIF_EXPR, req, ieq);
+ else
+ this->result_ = build_boolop (TRUTH_ORIF_EXPR, req, ieq);
+ }
+else if (tb1->isfloating () && tb1->ty != Tvector)
+ {
+ tree e1 = d_save_expr (build_expr (e->e1));
+ tree e2 = d_save_expr (build_expr (e->e2));
+ this->result_ = build_float_identity (code, e1, e2);
}
else if (tb1->ty == Tstruct)
{
diff --git a/gcc/testsuite/gdc.dg/runnable.d b/gcc/testsuite/gdc.dg/runnable.d
index ec172fae810..65c71e86292 100644
--- a/gcc/testsuite/gdc.dg/runnable.d
+++ b/gcc/testsuite/gdc.dg/runnable.d
@@ -1534,6 +1534,27 @@ void test286()
assert(0);
}
+/**/
+// https://bugzilla.gdcproject.org/show_bug.cgi?id=309
+
+void test309()
+{
+creal f1 = +0.0 + 0.0i;
+creal f2 = +0.0 - 0.0i;
+creal f3 = -0.0 + 0.0i;
+creal f4 = +0.0 + 0.0i;
+
+assert(f1 !is f2);
+assert(f1 !is f3);
+assert(f2 !is f3);
+assert(f1 is f4);
+
+assert(!(f1 is f2));
+assert(!(f1 is f3));
+assert(!(f2 is f3));
+assert(!(f1 !is f4));
+}
+
/**/
void main()
@@ -1571,6 +1592,7 @@ void main()
test273();
test285();
test286();
+test309();
printf("Success!\n");
}
--
2.19.1
