Re: [PATCH] Cheap enums.
[Thread Prev] | [Thread Next]
- Subject: Re: [PATCH] Cheap enums.
- From: Ori Bernstein <ori@xxxxxxxxxxxxxx>
- Reply-to: myrddin-dev@xxxxxxxxxxxxxx
- Date: Sun, 21 Jan 2018 15:20:34 -0800
- To: myrddin-dev@xxxxxxxxxxxxxx
- Cc: Quentin Carbonneaux <quentin@xxxxxx>
Not seeing anything obviously wrong, and the tests all pass. Landed, let's see if anyone screams. On Sun, 21 Jan 2018 22:46:47 +0000, Quentin Carbonneaux <quentin@xxxxxx> wrote: > This patch proposes to optimize the representation for > enum-like unions (i.e., unions with only nullary > constructors). > > Instead of stack-allocating them and blitting them for > copies, they are handled as simple integers. With the > current backend, this has the significant advantage > of making the register allocator able to registerize > those enums and simplifies the argument passing and > returning of enum types (instead of blitting to stack > and passing a pointer to the area, we can simply pass > the value). > > Benchmarks: https://c9x.me/paste/4Evh > Simple function with enums before: https://c9x.me/paste/COaf > Simple function with enums now: https://c9x.me/paste/3wXb > > Note: I really am not super confident in the implementation > so I'd advise paranoiac reviewing. It seems to pass the tests, > build a working mbld, and succeed at the benchmarks. > > --- > 6/isel.c | 12 ++++++++++-- > 6/simp.c | 30 ++++++++++++++---------------- > mbld/libs.myr | 2 +- > mi/flatten.c | 4 ---- > parse/parse.h | 4 +++- > parse/type.c | 32 +++++++++++++++++++++++++++++++- > parse/use.c | 4 +--- > 7 files changed, 60 insertions(+), 28 deletions(-) > > diff --git a/6/isel.c b/6/isel.c > index d7ec0c03..e6950967 100644 > --- a/6/isel.c > +++ b/6/isel.c > @@ -63,7 +63,7 @@ static Mode > tymode(Type *t) > { > /* FIXME: What should the mode for, say, structs be when we have no > - * intention of loading /through/ the pointer? For now, we'll just say it' > + * intention of loading /through/ the pointer? For now, we'll just say it's > * the pointer mode, since we expect to address through the pointer */ > t = tybase(t); > switch (t->type) { > @@ -128,6 +128,7 @@ static Loc * > loc(Isel *s, Node *n) > { > Node *v; > + Ucon *uc; > Loc *l; > > if (n->type == Ndecl) { > @@ -137,6 +138,10 @@ loc(Isel *s, Node *n) > case Ovar: > l = varloc(s, n); > break; > + case Oucon: > + uc = finducon(exprtype(n), n->expr.args[0]); > + l = loclit(uc->id, mode(n)); > + break; > case Olit: > v = n->expr.args[0]; > switch (v->lit.littype) { > @@ -858,6 +863,9 @@ selexpr(Isel *s, Node *n) > case Ovjmp: > selvjmp(s, n, args); > break; > + case Oucon: > + assert(isenum(tybase(exprtype(n)))); > + /* fallthrough */ > case Olit: > r = loc(s, n); > break; > @@ -936,7 +944,7 @@ selexpr(Isel *s, Node *n) > case Osubeq: case Omuleq: case Odiveq: case Omodeq: case Oboreq: > case Obandeq: case Obxoreq: case Obsleq: case Obsreq: case Omemb: > case Oslbase: case Osllen: case Ocast: case Outag: case Oudata: > - case Oucon: case Otup: case Oarr: case Ostruct: > + case Otup: case Oarr: case Ostruct: > case Oslice: case Oidx: case Osize: case Otupget: > case Obreak: case Ocontinue: > case Numops: > diff --git a/6/simp.c b/6/simp.c > index b43a083f..0a8e15f6 100644 > --- a/6/simp.c > +++ b/6/simp.c > @@ -338,8 +338,11 @@ uconid(Simp *s, Node *n) > Ucon *uc; > > n = rval(s, n, NULL); > - if (exprop(n) != Oucon) > + if (exprop(n) != Oucon) { > + if (isenum(tybase(exprtype(n)))) > + return n; > return load(addr(s, n, mktype(n->loc, Tyuint))); > + } > > uc = finducon(exprtype(n), n->expr.args[0]); > return word(uc->loc, uc->id); > @@ -539,12 +542,6 @@ lval(Simp *s, Node *n) > case Ostruct: r = rval(s, n, NULL); break; > case Oucon: r = rval(s, n, NULL); break; > case Oarr: r = rval(s, n, NULL); break; > - case Ogap: r = temp(s, n); break; > - > - /* not actually expressible as lvalues in syntax, but we generate them */ > - case Oudata: r = rval(s, n, NULL); break; > - case Outag: r = rval(s, n, NULL); break; > - case Otupget: r = rval(s, n, NULL); break; > default: > fatal(n, "%s cannot be an lvalue", opstr[exprop(n)]); > break; > @@ -840,17 +837,11 @@ simpucon(Simp *s, Node *n, Node *dst) > Node *r; > Type *ty; > Ucon *uc; > - size_t i, o; > + size_t o; > > /* find the ucon we're constructing here */ > - ty = tybase(n->expr.type); > - uc = NULL; > - for (i = 0; i < ty->nmemb; i++) { > - if (!strcmp(namestr(n->expr.args[0]), namestr(ty->udecls[i]->name))) { > - uc = ty->udecls[i]; > - break; > - } > - } > + ty = tybase(exprtype(n)); > + uc = finducon(ty, n->expr.args[0]); > if (!uc) > die("Couldn't find union constructor"); > > @@ -859,6 +850,13 @@ simpucon(Simp *s, Node *n, Node *dst) > else > tmp = temp(s, n); > > + if (isenum(ty)) { > + /* enums are treated as integers > + * by the backend */ > + append(s, set(tmp, n)); > + return tmp; > + } > + > /* Set the tag on the ucon */ > u = addr(s, tmp, mktype(n->loc, Tyuint)); > tag = mkintlit(n->loc, uc->id); > diff --git a/mbld/libs.myr b/mbld/libs.myr > index 49e2fc6a..f92d0f98 100644 > --- a/mbld/libs.myr > +++ b/mbld/libs.myr > @@ -22,7 +22,7 @@ pkg bld = > incs : byte[:][:] -> void) > ;; > > -const Abiversion = 16 > +const Abiversion = 17 > > const builtlib = {b, mt, dep, dyndep > var ldep, l, u > diff --git a/mi/flatten.c b/mi/flatten.c > index 1b95d0f9..03f3df1d 100644 > --- a/mi/flatten.c > +++ b/mi/flatten.c > @@ -631,10 +631,6 @@ lval(Flattenctx *s, Node *n) > case Oarr: r = rval(s, n); break; > case Ogap: r = temp(s, n); break; > > - /* not actually expressible as lvalues in syntax, but we generate them */ > - case Oudata: r = rval(s, n); break; > - case Outag: r = rval(s, n); break; > - case Otupget: r = rval(s, n); break; > default: > fatal(n, "%s cannot be an lvalue", opstr[exprop(n)]); > break; > diff --git a/parse/parse.h b/parse/parse.h > index 1bf00294..c7b94aa0 100644 > --- a/parse/parse.h > +++ b/parse/parse.h > @@ -1,4 +1,4 @@ > -#define Abiversion 16 > +#define Abiversion 17 > > typedef struct Srcloc Srcloc; > typedef struct Tysubst Tysubst; > @@ -158,6 +158,7 @@ struct Type { > }; > > char hasparams; /* cache for whether this type has params */ > + char isenum; /* Tyunion: see isenum(), it is lazily set there */ > char ishidden; /* Tyname: whether this is hidden or not */ > char ispkglocal; /* Tyname: whether this is package local or not */ > char isimport; /* Tyname: whether tyis type was imported. */ > @@ -470,6 +471,7 @@ Trait *mktrait(Srcloc l, Node *name, > int isproto); > Ucon *finducon(Type *t, Node *name); > int isstacktype(Type *t); > +int isenum(Type *t); > int istysigned(Type *t); > int istyunsigned(Type *t); > int istyfloat(Type *t); > diff --git a/parse/type.c b/parse/type.c > index c31fbbe3..8674d8c2 100644 > --- a/parse/type.c > +++ b/parse/type.c > @@ -44,7 +44,37 @@ char stackness[] = { > int > isstacktype(Type *t) > { > - return stackness[tybase(t)->type]; > + t = tybase(t); > + if (t->type == Tyunion) > + return !isenum(t); > + return stackness[t->type]; > +} > + > +int > +isenum(Type *t) > +{ > + size_t i; > + char isenum; > + > + assert(t->type == Tyunion); > + > + /* t->isenum is lazily set: > + * a value of 0 means that it was not computed, > + * a value of 1 means that the type is an enum > + * (i.e., it only has nullary constructors) > + * a value of 2 means that the type is not an enum > + */ > + if (t->isenum == 0) { > + /* initialize it */ > + isenum = 1; > + for (i = 0; i < t->nmemb; i++) > + if (t->udecls[i]->etype) { > + isenum = 2; > + break; > + } > + t->isenum = isenum; > + } > + return t->isenum == 1; > } > > Type * > diff --git a/parse/use.c b/parse/use.c > index 605b3be2..c40c9bca 100644 > --- a/parse/use.c > +++ b/parse/use.c > @@ -361,9 +361,7 @@ rdtrait(FILE *fd, Trait **dest, Type *ty) > } > } > > -/* Writes types to a file. Errors on > - * internal only types like Tyvar that > - * will not be meaningful in another file */ > +/* Reads types from a file. */ > static Type * > tyunpickle(FILE *fd) > { > -- > 2.16.0 > > -- Ori Bernstein
[PATCH] Cheap enums. | Quentin Carbonneaux <quentin@xxxxxx> |
- Prev by Date: [PATCH] Cheap enums.
- Next by Date: Re: [PATCH] New auto operator.
- Previous by thread: [PATCH] Cheap enums.
- Next by thread: [PATCH] Improve error message for mistmatched formats.
- Index(es):