lynn/chibicc

Support assignment between structs

Closed this issue · 7 comments

lynn commented

If x and y are structs of the same type, y = x should copy all the fields, maybe using something like memcpy.

A test that should pass:

  assert(9, ({ struct {int a; int b; int c;} x = {2,3,4}, y = {0,0,0}; y = x; y.a + y.b + y.c; }),
              "struct {int a; int b; int c;} x = {2,3,4}, y = {0,0,0}; y = x; y.a + y.b + y.c;");

Right now it returns 2 (so we are only copying the first field).

hi, i'm interested to help implement this! do you have any pointers?

lynn commented

Hi @youbitchoc! I think what needs to happen is, this bit of code in codegen.c...

  case ND_ASSIGN:
    gen(node->rhs, depth);
    fix_bool(node->ty);
    gen_lval(node->lhs, depth + 2);
    store(node->ty);
    return;

has to become a bit smarter. It should check if ty->kind == TY_STRUCT and if so (generate code to) do a memcpy of ty->size bytes. gen_lval(node->rhs, depth) is a pointer to the source and node->lhs is a pointer to the target.

For the memcpy, maybe it can be an "intrinsic" like how sign extension is handled (see need_sext_helper).

is it possible to modify the store function to handle ty->sizes other than 1 and 2?

// Expects "newval addr" on the stack and leaves "newval".
static void store(Type *ty) {
  if (ty->size == 1) {
    op(STA | flag_k);
    op(POP2);
  } else {
    op(STA2 | flag_k);
    op(POP2);
  }
}

the opcodes will probably be much more complex though...

lynn commented

It might be a bad idea to solve this at the level of store. That function handles a scenario where the new value is on the stack. But in an assignment between structs like y = x, we don't want the value (i.e. all the fields) of x to be put on the stack, only its address (hence my suggestion to use gen_lval(node->rhs) in some way).

mm! i tried implementing it and realize now that contorting the stack is horribly inefficient

since struct size is known at compile time, could it be better to bake in the "memcpy" instead of outsourcing to an intrinsic?

lynn commented

Possibly, yes! That sounds worth exploring. But I wouldn't shy away from doing it the simple way first.

got it half working, made a pr at #38