Bug 57 - Comparing small structs fails

gcc/d/ChangeLog

@@ -1,3 +1,9 @@
+2013-06-17  Iain Buclaw  <ibuclaw@gdcproject.org>
+
+	* d-codegen.cc(build_struct_memcmp): New function.
+	* d-elem.cc(IdentityExp::toElem): Use build_struct_memcmp for field
+	comparisons of small structs.
+
 2013-06-13  Iain Buclaw  <ibuclaw@gdcproject.org>
 
 	* d-codegen.cc(make_temp): New function.

gcc/d/d-codegen.cc

@@ -1314,35 +1314,35 @@ build_two_field_type (tree t1, tree t2, Type *type, const char *n1, const char *
   return rec_type;
 }
 
-// Create a SAVE_EXPR if T might have unwanted side effects if referenced
+// Create a SAVE_EXPR if EXP might have unwanted side effects if referenced
 // more than once in an expression.
 
 tree
-make_temp (tree t)
+make_temp (tree exp)
 {
-  if (TREE_CODE (t) == CALL_EXPR
-      || TREE_CODE (TREE_TYPE (t)) != ARRAY_TYPE)
-    return save_expr (t);
+  if (TREE_CODE (exp) == CALL_EXPR
+      || TREE_CODE (TREE_TYPE (exp)) != ARRAY_TYPE)
+    return save_expr (exp);
   else
-    return stabilize_reference (t);
+    return stabilize_reference (exp);
 }
 
 tree
-maybe_make_temp (tree t)
+maybe_make_temp (tree exp)
 {
-  if (d_has_side_effects (t))
-    return make_temp (t);
+  if (d_has_side_effects (exp))
+    return make_temp (exp);
 
-  return t;
+  return exp;
 }
 
-// Return TRUE if T can not be evaluated multiple times (i.e., in a loop body)
+// Return TRUE if EXP can not be evaluated multiple times (i.e., in a loop body)
 // without unwanted side effects.
 
 bool
-d_has_side_effects (tree expr)
+d_has_side_effects (tree exp)
 {
-  tree t = STRIP_NOPS (expr);
+  tree t = STRIP_NOPS (exp);
 
   // SAVE_EXPR is safe to reference more than once, but not to
   // expand in a loop.
@@ -1538,6 +1538,79 @@ d_mark_read (tree exp)
   return exp;
 }
 
+// Build equality expression between two RECORD_TYPES T1 and T2.
+// CODE is the EQ_EXPR or NE_EXPR comparison.
+// SD is the front-end struct type.
+
+tree
+build_struct_memcmp (tree_code code, StructDeclaration *sd, tree t1, tree t2)
+{
+  tree_code tcode = (code == EQ_EXPR) ? TRUTH_ANDIF_EXPR : TRUTH_ORIF_EXPR;
+  tree tmemcmp = NULL_TREE;
+
+  // Let backend take care of empty struct or union comparisons.
+  if (!sd->fields.dim || sd->isUnionDeclaration())
+    {
+      tmemcmp = d_build_call_nary (builtin_decl_explicit (BUILT_IN_MEMCMP), 3,
+				   build_address (t1), build_address (t2),
+				   size_int (sd->structsize));
+
+      return build_boolop (code, tmemcmp, integer_zero_node);
+    }
+
+  for (size_t i = 0; i < sd->fields.dim; i++)
+    {
+      VarDeclaration *vd = sd->fields[i];
+      tree sfield = vd->toSymbol()->Stree;
+
+      tree t1ref = component_ref (t1, sfield);
+      tree t2ref = component_ref (t2, sfield);
+      tree tcmp;
+
+      if (vd->type->ty == Tstruct)
+	{
+	  // Compare inner data structures.
+	  StructDeclaration *decl = ((TypeStruct *) vd->type)->sym;
+	  tcmp = build_struct_memcmp (code, decl, t1ref, t2ref);
+	}
+      else
+	{
+	  tree stype = vd->type->toCtype();
+	  enum machine_mode mode = int_mode_for_mode (TYPE_MODE (stype));
+
+	  if (vd->type->isintegral())
+	    {
+	      // Integer comparison, no special handling required.
+	      tcmp = build_boolop (code, t1ref, t2ref);
+	    }
+	  else if (mode != BLKmode)
+	    {
+	      // Compare field bits as their corresponding integer type.
+	      //   *((T*) &t1) == *((T*) &t2)
+	      tree tmode = lang_hooks.types.type_for_mode (mode, 1);
+
+	      t1ref = build_vconvert (tmode, t1ref);
+	      t2ref = build_vconvert (tmode, t2ref);
+
+	      tcmp = build_boolop (code, t1ref, t2ref);
+	    }
+	  else
+	    {
+	      // Simple memcmp between types.
+	      tcmp = d_build_call_nary (builtin_decl_explicit (BUILT_IN_MEMCMP), 3,
+					build_address (t1ref), build_address (t2ref),
+					TYPE_SIZE_UNIT (stype));
+
+	      tcmp = build_boolop (code, tcmp, integer_zero_node);
+	    }
+	}
+
+      tmemcmp = (tmemcmp) ? build_boolop (tcode, tmemcmp, tcmp) : tcmp;
+    }
+
+  return tmemcmp;
+}
+
 // Cast EXP (which should be a pointer) to TYPE * and then indirect.  The
 // back-end requires this cast in many cases.
 

gcc/d/d-codegen.h

@@ -147,6 +147,8 @@ extern tree d_mark_used (tree exp);
 extern tree d_mark_read (tree exp);
 extern tree build_address (tree exp);
 
+extern tree build_struct_memcmp (tree_code code, StructDeclaration *sd, tree arg0, tree arg1);
+
 // Routines to handle variables that are references.
 extern bool decl_reference_p (Declaration *decl);
 extern tree declaration_type (Declaration *decl);

gcc/d/d-elem.cc

@@ -46,47 +46,63 @@ IdentityExp::toElem (IRState *irs)
   Type *tb1 = e1->type->toBasetype();
   Type *tb2 = e2->type->toBasetype();
 
-  tree_code code = op == TOKidentity ? EQ_EXPR : NE_EXPR;
+  tree_code code = (op == TOKidentity) ? EQ_EXPR : NE_EXPR;
 
-  if (tb1->ty == Tstruct || tb1->isfloating())
+  if ((tb1->ty == Tsarray || tb1->ty == Tarray)
+      && (tb2->ty == Tsarray || tb2->ty == Tarray))
     {
-      tree size;
-      if (tb1->isfloating())
-	{
-	  // Assume all padding is at the end of the type.
-	  size = build_integer_cst (TYPE_PRECISION (e1->type->toCtype()) / BITS_PER_UNIT);
-	}
-      else
-	size = build_integer_cst (e1->type->size());
+      // Convert arrays to D array types.
+      return build2 (code, type->toCtype(), irs->toDArray (e1), irs->toDArray (e2));
+    }
+  else if (tb1->isfloating())
+    {
+      tree t1 = e1->toElem (irs);
+      tree t2 = e2->toElem (irs);
+      // Assume all padding is at the end of the type.
+      tree size = build_integer_cst (TYPE_PRECISION (e1->type->toCtype()) / BITS_PER_UNIT);
 
-      // Do bit compare.
+      // Do bit compare of floats.
       tree tmemcmp = d_build_call_nary (builtin_decl_explicit (BUILT_IN_MEMCMP), 3,
-					build_address (e1->toElem (irs)),
-					build_address (e2->toElem (irs)),
-					size);
+					build_address (t1), build_address (t2), size);
 
       return build_boolop (code, tmemcmp, integer_zero_node);
     }
-  else if ((tb1->ty == Tsarray || tb1->ty == Tarray)
-	   && (tb2->ty == Tsarray || tb2->ty == Tarray))
-    {
-      return build2 (code, type->toCtype(),
-		     irs->toDArray (e1), irs->toDArray (e2));
-    }
-  else
+  else if (tb1->ty == Tstruct)
     {
-      // For operands of other types, identity is defined as being the same as equality.
       tree t1 = e1->toElem (irs);
       tree t2 = e2->toElem (irs);
 
-      if (type->iscomplex())
+      if (TYPE_MODE (TREE_TYPE (t1)) != BLKmode)
 	{
+	  // Bitwise comparison of small structs not returned in memory may
+	  // not work due to data holes loosing its zero padding upon return.
+	  // Instead do field-by-field comparison of the two structs.
+	  StructDeclaration *sd = ((TypeStruct *) tb1)->sym;
+	  gcc_assert (d_types_same (tb1, tb2));
+
+	  // Make temporaries to prevent multiple evaluations.
 	  t1 = maybe_make_temp (t1);
 	  t2 = maybe_make_temp (t2);
+
+	  return build_struct_memcmp (code, sd, t1, t2);
+	}
+      else
+	{
+	  // Do bit compare of structs.
+	  tree size = build_integer_cst (e1->type->size());
+
+	  tree tmemcmp = d_build_call_nary (builtin_decl_explicit (BUILT_IN_MEMCMP), 3,
+					    build_address (t1), build_address (t2), size);
+
+	  return build_boolop (code, tmemcmp, integer_zero_node);
 	}
+    }
+  else
+    {
+      // For operands of other types, identity is defined as being the same as equality.
+      tree tcmp = build_boolop (code, e1->toElem (irs), e2->toElem (irs));
 
-      tree t_cmp = build_boolop (code, t1, t2);
-      return d_convert (type->toCtype(), t_cmp);
+      return d_convert (type->toCtype(), tcmp);
     }
 }
 
@@ -100,7 +116,7 @@ EqualExp::toElem (IRState *irs)
 
   if (tb1->ty == Tstruct)
     {
-      // Do bit compare of struct's
+      // Do bit compare of structs
       tree tmemcmp = d_build_call_nary (builtin_decl_explicit (BUILT_IN_MEMCMP), 3,
 					build_address (e1->toElem (irs)),
 					build_address (e2->toElem (irs)),
@@ -116,7 +132,7 @@ EqualExp::toElem (IRState *irs)
 
       if ((t1elem->isintegral() || t1elem->ty == Tvoid) && t1elem->ty == t2elem->ty)
 	{
-	  // Optimize comparisons of arrays of basic types.
+	  // Optimise comparisons of arrays of basic types.
 	  // For arrays of integers/characters, and void[], replace _adEq2 call with:
 	  //     e1 == e2  =>  e1.length == e2.length && memcmp (e1.ptr, e2.ptr, size) == 0;
 	  //     e1 != e2  =>  e1.length != e2.length || memcmp (e1.ptr, e2.ptr, size) != 0;
@@ -219,17 +235,9 @@ EqualExp::toElem (IRState *irs)
     }
   else
     {
-      tree t1 = e1->toElem (irs);
-      tree t2 = e2->toElem (irs);
-
-      if (type->iscomplex())
-	{
-	  t1 = maybe_make_temp (t1);
-	  t2 = maybe_make_temp (t2);
-	}
+      tree tcmp = build_boolop (code, e1->toElem (irs), e2->toElem (irs));
 
-      tree t_cmp = build_boolop (code, t1, t2);
-      return d_convert (type->toCtype(), t_cmp);
+      return d_convert (type->toCtype(), tcmp);
     }
 }
 
@@ -370,14 +378,7 @@ AndAndExp::toElem (IRState *irs)
       tree t1 = convert_for_condition (e1->toElem (irs), e1->type);
       tree t2 = convert_for_condition (e2->toElem (irs), e2->type);
 
-      if (type->iscomplex())
-	{
-	  t1 = maybe_make_temp (t1);
-	  t2 = maybe_make_temp (t2);
-	}
-
-      tree t = build_boolop (TRUTH_ANDIF_EXPR, t1, t2);
-      return d_convert (type->toCtype(), t);
+      return d_convert (type->toCtype(), build_boolop (TRUTH_ANDIF_EXPR, t1, t2));
     }
   else
     {
@@ -395,14 +396,7 @@ OrOrExp::toElem (IRState *irs)
       tree t1 = convert_for_condition (e1->toElem (irs), e1->type);
       tree t2 = convert_for_condition (e2->toElem (irs), e2->type);
 
-      if (type->iscomplex())
-	{
-	  t1 = maybe_make_temp (t1);
-	  t2 = maybe_make_temp (t2);
-	}
-
-      tree t = build_boolop (TRUTH_ORIF_EXPR, t1, t2);
-      return d_convert (type->toCtype(), t);
+      return d_convert (type->toCtype(), build_boolop (TRUTH_ORIF_EXPR, t1, t2));
     }
   else
     {

gcc/d/dfrontend/statement.c

@@ -667,6 +667,12 @@ Statement *CompoundStatement::semantic(Scope *sc)
                     }
                 }
             }
+            else
+            {
+                // Remove NULL statement from list.
+                statements->remove(i);
+                continue;
+            }
         }
         i++;
     }
@@ -4423,8 +4429,9 @@ Statement *TryCatchStatement::syntaxCopy()
 Statement *TryCatchStatement::semantic(Scope *sc)
 {
     body = body->semanticScope(sc, NULL /*this*/, NULL);
+    assert(body);
 
-    /* Even if body is NULL, still do semantic analysis on catches
+    /* Even if body is empty, still do semantic analysis on catches
      */
     for (size_t i = 0; i < catches->dim; i++)
     {   Catch *c = (*catches)[i];
@@ -4441,28 +4448,6 @@ Statement *TryCatchStatement::semantic(Scope *sc)
         }
     }
 
-#ifdef IN_GCC
-    if (!body || !body->hasCode())
-    {
-        for (size_t i = 0; i < catches->dim; i++)
-        {
-            Catch *c = (*catches)[i];
-            if (!c->handler || !c->handler->comeFrom())
-            {
-                catches->remove(i);
-                --i;
-            }
-        }
-        if (catches->dim == 0)
-            return NULL;
-    }
-#else
-    if (!body || !body->hasCode())
-    {
-        return NULL;
-    }
-#endif
-
     /* If the try body never throws, we can eliminate any catches
      * of recoverable exceptions.
      */
@@ -4484,7 +4469,7 @@ Statement *TryCatchStatement::semantic(Scope *sc)
     }
 
     if (catches->dim == 0)
-        return body;
+        return (body->hasCode()) ? body : NULL;
 
     return this;
 }