Skip to content

Commit

Permalink
Some more small bug fixes.
Browse files Browse the repository at this point in the history
  • Loading branch information
Warren Toomey committed Dec 14, 2019
1 parent 96ae340 commit 242fcf3
Show file tree
Hide file tree
Showing 7 changed files with 163 additions and 17 deletions.
92 changes: 87 additions & 5 deletions 62_Cleanup/Readme.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,93 @@
# Part 62: Code Cleanup

This version of the compiler is functionally the same as in part 60.
I am using this part to fix up comments, do a bit of code cleanup,
rename some functions and variables etc.
This version of the compiler is essentially the same as in part 60.
I am using this part to fix up comments, fix up bugs, do a bit of
code cleanup, rename some functions and variables etc.

The files here may change over time as I spot more things to fix, but
it will always be functionally equivalent to part 60.
## Some Small Bugfixes

For the changes to the compiler that I'm planning, I need to be able
to put structs into structs. Therefore, I should be able to do:

```
printf("%d\n", thing.member1.age_in_years);
```

where `thing` is a struct, but it has a `member1` which is of type struct.
To do this, we need to find the offset of `member1` from the base of
`thing`, then find the offset of `age_in_years` from the previous offset.

However, the code to do this expects the things on the left-hand side of
the '.' token to be a variable which has a symbol table entry and thus
a fixed location in memory. We need to fix this to deal with the situation
where the left-hand side of the '.' token is an offset that has already
been calculated.

Fortunately, this was quite easy to do. We don't have to change the
parser code, but let's look at what is already there. In `member_access()`
in `expr.c`:

```
// Check that the left AST tree is a struct or union.
// If so, change it from an A_IDENT to an A_ADDR so that
// we get the base address, not the value at this address.
if (!withpointer) {
if (left->type == P_STRUCT || left->type == P_UNION)
left->op = A_ADDR;
```

We mark the left-hand AST tree as A_ADDR (instead of A_IDENT) to say
that we need the base address of it, not the value at this address.

Now we need to fix the code generation. When we get an A_ADDR AST node,
we either have a variable whose address we need (e.g. `thing` in
`thing.member1`), or our child tree has the pre-calculated offset
(e.g. the offset of `member1` in `member1.age_in_years). So in `genAST()`
in `gen.c`, we do:

```
case A_ADDR:
// If we have a symbol, get its address. Otherwise,
// the left register already has the address because
// it's a member access
if (n->sym != NULL)
return (cgaddress(n->sym));
else
return (leftreg);
```

That should be all, but we have one more fix. The code to work out the
alignment of types doesn't deal with structs inside structs, only
scalar types inside structs. So, I've modified `cgalign()` in `cg.c`
as follows:

```
// Given a scalar type, an existing memory offset
// (which hasn't been allocated to anything yet)
// and a direction (1 is up, -1 is down), calculate
// and return a suitably aligned memory offset
// for this scalar type. This could be the original
// offset, or it could be above/below the original
int cgalign(int type, int offset, int direction) {
int alignment;
// We don't need to do this on x86-64, but let's
// align chars on any offset and align ints/pointers
// on a 4-byte alignment
switch (type) {
case P_CHAR:
break;
default:
// Align whatever we have now on a 4-byte alignment.
// I put the generic code here so it can be reused elsewhere.
alignment = 4;
offset = (offset + direction * (alignment - 1)) & ~(alignment - 1);
}
return (offset);
}
```

Everything but P_CHAR gets aligned on a 4-byte, including structs and unions.

## What's Next

Expand Down
14 changes: 4 additions & 10 deletions 62_Cleanup/cg.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,19 +56,13 @@ int cgalign(int type, int offset, int direction) {
// on a 4-byte alignment
switch (type) {
case P_CHAR:
return (offset);
case P_INT:
case P_LONG:
break;
default:
if (!ptrtype(type))
fatald("Bad type in cg_align:", type);
// Align whatever we have now on a 4-byte alignment.
// I put the generic code here so it can be reused elsewhere.
alignment = 4;
offset = (offset + direction * (alignment - 1)) & ~(alignment - 1);
}

// Here we have an int or a long. Align it on a 4-byte offset.
// I put the generic code here so it can be reused elsewhere.
alignment = 4;
offset = (offset + direction * (alignment - 1)) & ~(alignment - 1);
return (offset);
}

Expand Down
8 changes: 7 additions & 1 deletion 62_Cleanup/gen.c
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,13 @@ int genAST(struct ASTnode *n, int iflabel, int looptoplabel,
cgreturn(leftreg, Functionid);
return (NOREG);
case A_ADDR:
return (cgaddress(n->sym));
// If we have a symbol, get its address. Otherwise,
// the left register already has the address because
// it's a member access
if (n->sym != NULL)
return (cgaddress(n->sym));
else
return (leftreg);
case A_DEREF:
// If we are an rvalue, dereference to get the value we point at,
// otherwise leave it for A_ASSIGN to store through the pointer
Expand Down
2 changes: 1 addition & 1 deletion 62_Cleanup/scan.c
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ static int keyword(char *s) {
char *Tstring[] = {
"EOF", "=", "+=", "-=", "*=", "/=", "%=",
"?", "||", "&&", "|", "^", "&",
"==", "!=", ",", ">", "<=", ">=", "<<", ">>",
"==", "!=", "<", ">", "<=", ">=", "<<", ">>",
"+", "-", "*", "/", "%", "++", "--", "~", "!",
"void", "char", "int", "long",
"if", "else", "while", "for", "return",
Expand Down
58 changes: 58 additions & 0 deletions 62_Cleanup/tests/input150.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
#include <stdio.h>
#include <stdlib.h>

struct Svalue {
char *thing;
int vreg;
int intval;
};

struct IR {
int label;
int op;
struct Svalue dst;
struct Svalue src1;
struct Svalue src2;
int jmplabel;
};

struct foo {
int a;
int b;
struct Svalue *c;
int d;
};

struct IR *fred;
struct foo jane;

int main() {
fred= (struct IR *)malloc(sizeof(struct IR));
fred->label= 1;
fred->op= 2;
fred->dst.thing= NULL;
fred->dst.vreg=3;
fred->dst.intval=4;
fred->src1.thing= NULL;
fred->src1.vreg=5;
fred->src1.intval=6;
fred->src2.thing= NULL;
fred->src2.vreg=7;
fred->src2.intval=8;
fred->jmplabel= 9;

printf("%d %d %d\n", fred->label, fred->op, fred->dst.vreg);
printf("%d %d %d\n", fred->dst.intval, fred->src1.vreg, fred->src1.intval);
printf("%d %d %d\n\n", fred->src2.vreg, fred->src2.intval, fred->jmplabel);

jane.c= (struct Svalue *)malloc(sizeof(struct Svalue));
jane.a= 1; jane.b= 2; jane.d= 4;
jane.c->thing= "fish";
jane.c->vreg= 3;
jane.c->intval= 5;

printf("%d %d %d\n", jane.a, jane.b, jane.c->vreg);
printf("%d %d %s\n", jane.d, jane.c->intval, jane.c->thing);

return(0);
}
Empty file modified 62_Cleanup/tests/mktests
100644 → 100755
Empty file.
6 changes: 6 additions & 0 deletions 62_Cleanup/tests/out.input150.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
1 2 3
4 5 6
7 8 9

1 2 3
4 5 fish

0 comments on commit 242fcf3

Please sign in to comment.