[gccsdk] Objasm record layouts revisited

John Tytgat John.Tytgat at aaug.net
Sun Nov 8 08:23:11 PST 2009


In message <4e7fd8b250.belles at ivy.at.home>
          Christopher Martin <belles at internode.on.net> wrote:

> A month ago, I offered a tiny patch to objasm intended to allow
> expressions with record layout symbols. At the time, I did not
> appreciate all the issues with record layouts.
> 
> I have since revisited the code and made another attempt to resolve the
> long-outstanding issues. More work is probably required, but I think
> this attempt is a good start. Sorry it has taken a month to get this
> far. It has been many years since I had such difficulty understanding
> another's code.

Many thanks for your efforts.  Yes, the objasm (or asasm it is now
called) sources are not the most easy ones to understand.

> I have attached a diff file and a zip archive of my modified objasm
> sources from the 4.1.1 1b release. Note that my files are organised
> according to RISC OS convention; I hope this is okay.

The RISC OS filename convention gave a little bit of trouble but that
was easy to overcome.

But I had much more trouble that this is a context diff and not a unified
one (diff option -u) which makes it much more difficult to apply,
moreover when the source on trunk have been updated since last release.
And those changes were made in the same area as your changes.

In fact I'm not even sure if I got your changes applied correctly as
the previous failing tests_fail/register_based_map_fail.s is still
not processing correctly.  Or am I assuming something wrong here ?

May I suggest the following ? Assuming you want to develop asasm
on RISC OS, please grap a copy of the RISC OS subversion port at
<URL:http://www.cp15.org/versioncontrol/> and in a TaskWindow
go to a directory where you want to checkout the latest version
by doing:

* svn co svn://svn.riscos.info/gccsdk/trunk/gcc4/riscos/asasm

Despite the sources are checked in as foo.c/foo.h filenames in our
repository, by default on RISC OS you'll end up with c.foo and h.foo
files so you'll probably just have to add a RISC OS makefile (which
I guess you already have).

And when you want to submit your work, just a "svn diff > my_patch"
will do (unified diff output is standard with svn).

I've attached the differences I made using your patch and those are
made against what we have now in trunk.  Could you verify if those
are still correct and if so, which test case they solve (as I don't
see failing tests_fail/register_based_map_fail.s being solved) ?

Thanks,
John.
-- 
John Tytgat, in his comfy chair at home                                 BASS
John.Tytgat at aaug.net                             ARM powered, RISC OS driven
-------------- next part --------------
Index: code.c
===================================================================
--- code.c	(revision 4189)
+++ code.c	(working copy)
@@ -204,6 +204,45 @@
     error (ErrorSerious, FALSE, "Internal codeBool: overflow");
 }
 
+void
+codeValue (const Value *value)
+{
+  switch (value->Tag.t)
+    {
+      case ValueInt:
+        codeInt (value->ValueInt.i);
+        break;
+      case ValueFloat:
+        codeFloat (value->ValueFloat.f);
+        break;
+      case ValueString:
+        codeString (value->ValueString.len, value->ValueString.s);
+        break;
+      case ValueBool:
+        codeBool (value->ValueBool.b);
+        break;
+      case ValueCode:
+        switch (value->ValueCode.c->Tag)
+          {
+            case CodeSymbol:
+              codePosition (value->ValueCode.c->CodeSymbol.symbol);
+              break;
+            default:
+              error (ErrorSerious, FALSE, "code::codeValue received a non-CodeSymbol ValueCode");
+          }
+        break;
+      case ValueLateLabel:
+        codeSymbol (value->ValueLate.late->symbol);
+        codeInt (value->ValueLate.i);
+        codeOperator (Op_add);
+        break;
+      case ValueAddr:
+        codeInt (value->ValueAddr.i);
+        break;
+      default:
+        error (ErrorSerious, FALSE, "code::codeValue received a Value of unhandled type");
+    }
+}
 
 /* TRUE if codeEvalLowest succeeded */
 static BOOL
Index: code.h
===================================================================
--- code.h	(revision 4189)
+++ code.h	(working copy)
@@ -75,6 +75,7 @@
 void codeString (int len, const char *str);
 void codeFloat (FLOAT value);
 void codeBool (BOOL value);
+void codeValue (const Value *value);
 
 Value codeEvalLow (ValueTag legal, int size, Code *program);
 Value codeEval (ValueTag legal);
Index: m_cpuctrl.c
===================================================================
--- m_cpuctrl.c	(revision 4189)
+++ m_cpuctrl.c	(working copy)
@@ -242,7 +242,7 @@
   WORD ir = cc & ~1, ir2 = 0;	/* bit 0 set to indicate ADRL */
   Value im;
   ir |= DST_OP (getCpuReg ());
-  ir |= LHS_OP (15) | IMM_RHS;	/* pc */
+  ir |= LHS_OP (15) | IMM_RHS;	/* pc, may have to undo this later */
   skipblanks ();
   if (inputLook () == ',')
     {
@@ -252,10 +252,10 @@
   else
     error (ErrorError, TRUE, "%sdst", InsertCommaAfter);
   /* The label will expand to either a field in a based map or a PC-relative 
-   * expression */
+     expression.  */
   exprBuild ();
   /* Try the field first */
-  im = exprEval (ValueAddr);
+  im = exprEval (ValueInt | ValueCode | ValueLateLabel | ValueAddr);
   switch (im.Tag.t)
     {
     case ValueAddr:
@@ -270,9 +270,11 @@
       break;
     default:
       /* Not a field, so append " - . - 8" to the expression, and treat it as
-       * PC-relative */
+         PC-relative.  */
       /* TODO: It may also be a late field but there's no simple way of
-       * handling this when performing relocation */
+         handling this when performing relocation.  */
+      codeInit ();
+      codeValue (&im);
       codePosition (areaCurrentSymbol);	/* It's relative */
       codeOperator (Op_sub);
       codeInt (8);
Index: m_cpumem.c
===================================================================
--- m_cpumem.c	(revision 4189)
+++ m_cpumem.c	(working copy)
@@ -228,6 +228,8 @@
 	  /* The previous evaluation will have left its result in the current
 	   * program. Append " - . - 8" to the program so we can calculate the
 	   * offset of the label from the current position. */
+          codeInit ();
+          codeValue (&offset);
 	  codePosition (areaCurrentSymbol);
 	  codeOperator (Op_sub);
 	  codeInt (8);
Index: value.c
===================================================================
--- value.c	(revision 4189)
+++ value.c	(working copy)
@@ -265,13 +265,20 @@
 	printf("<%s> (bool)\n", v->ValueBool.b ? "TRUE" : "FALSE");
 	break;
       case ValueCode:
-	printf("(code)\n");
+        printf("<%d , %p> (code)\n", v->ValueCode.len, v->ValueCode.c);
 	break;
       case ValueLateLabel:
-	printf("(late label)\n");
+        printf("<%d , %p: <%d , %p: <&%x , %d , %d , %.*s>>> (late label)\n",
+               v->ValueLate.i, v->ValueLate.late,
+               v->ValueLate.late->factor, v->ValueLate.late->symbol,
+               v->ValueLate.late->symbol->type, v->ValueLate.late->symbol->declared,
+               v->ValueLate.late->symbol->used,
+               v->ValueLate.late->symbol->len, v->ValueLate.late->symbol->str);
+        printf("\t");
+        valuePrint(&v->ValueLate.late->symbol->value);
 	break;
       case ValueAddr:
-	printf("(addr)\n");
+        printf("<r%d,#%d> (addr)\n", v->ValueAddr.r, v->ValueAddr.i);
 	break;
       default:
 	printf("???\n");
Index: value.h
===================================================================
--- value.h	(revision 4189)
+++ value.h	(working copy)
@@ -119,4 +119,5 @@
 #ifdef DEBUG
 void valuePrint(const Value *v);
 #endif
+#define valuePrint(v)
 #endif
Index: eval.c
===================================================================
--- eval.c	(revision 4189)
+++ eval.c	(working copy)
@@ -123,10 +123,19 @@
 	  lvalue->ValueFloat.f += rvalue->ValueFloat.f;
 	  return TRUE;
 	}
-      if (lvalue->Tag.t == ValueAddr && rvalue->Tag.t == ValueInt)
+      /* Allow more flexible operations with ValueAddr labels.  */
+      if ((lvalue->Tag.t & (ValueInt | ValueAddr)) && (rvalue->Tag.t & (ValueInt | ValueAddr)))
 	{
-	  lvalue->ValueAddr.i += rvalue->ValueInt.i;
+          /* ValueInt and ValueAddr have field i in the same place.  */
+          lvalue->ValueInt.i += rvalue->ValueInt.i;
+          if (lvalue->Tag.t == ValueInt && rvalue->Tag.t == ValueAddr)
+            {
+              /* Transform lvalue from a ValueInt into a ValueAddr like rvalue */
+              lvalue->Tag.t = ValueAddr;
+              lvalue->ValueAddr.r = rvalue->ValueAddr.r;
+            }
 	  return TRUE;
+          /* Note that I have relaxed the requirement for identical base registers */
 	}
       if ((lvalue->Tag.t & (ValueInt | ValueLateLabel)) &&
 	  (rvalue->Tag.t & (ValueInt | ValueLateLabel)))
@@ -160,15 +169,19 @@
 	  lvalue->ValueFloat.f -= rvalue->ValueFloat.f;
 	  return TRUE;
 	}
-      if (lvalue->Tag.t == ValueAddr && (rvalue->Tag.t & (ValueInt | ValueAddr)))
+      /* Allow more flexible operations with ValueAddr labels.  */
+      if ((lvalue->Tag.t & (ValueInt | ValueAddr)) && (rvalue->Tag.t & (ValueInt | ValueAddr)))
 	{
-	  if (rvalue->Tag.t == ValueAddr &&
-	      lvalue->ValueAddr.r != rvalue->ValueAddr.r)
-	    return FALSE;
-	  lvalue->ValueAddr.i -= rvalue->ValueInt.i;
-	  if (rvalue->Tag.t == ValueAddr)
-	    lvalue->Tag.t = ValueInt;
+          /* ValueInt and ValueAddr have field i in the same place */
+          lvalue->ValueInt.i -= rvalue->ValueInt.i;
+          if (lvalue->Tag.t == ValueInt && rvalue->Tag.t == ValueAddr)
+            {
+              /* Transform lvalue from a ValueInt into a ValueAddr like rvalue */
+              lvalue->Tag.t = ValueAddr;
+              lvalue->ValueAddr.r = rvalue->ValueAddr.r;
+            }
 	  return TRUE;
+          /* Note that I have relaxed the requirement for identical base registers */
 	}
       if (!(lvalue->Tag.t & (ValueInt | ValueLateLabel)) ||
 	  !(rvalue->Tag.t & (ValueInt | ValueLateLabel)))


More information about the gcc mailing list