search

ykjit / yk

yk packages
https://ykjit.github.io/yk/
Other
29 stars 7 forks source link

Terminology tweaks #1289

Closed ltratt closed 3 months ago

ltratt commented 3 months ago

This is a slightly half-hearted PR where I wanted to do something big, got bogged down in details, and then decided to salvage something minor.

The first commit somewhat addresses the comment from #1284 that the X86 structs weren't well named.

The second commit starts to make the register allocation terminology a bit clearer. Doing so has proven unintentionally useful: it makes clear that the RegisterAllocator trait fundamentally assumes that SSA variables have a single location, which means one can't express "store in a reg, spill to stack, reload from a reg" and so on. Doing something about that is going to be quite some work, hence this not-even-halfway-house PR.

vext01 commented 3 months ago

I'm luke warm on the second commit.

I do agree that "register allocation" is a bad name, given that you typically allocate stack too as part of that process. "storage allocation" might have been a better name, but hey ho.

I don't really have a problem with LocalAllocation. You can allocate a register or a stack slot for a local. So it seems accurate.

That said, VarLocation is fine too.

Unless any of that changes your mind, I'm happy to merge.

ltratt commented 3 months ago

One of the challenges is that in a "real" register allocator, the location of a variable changes over time (as you spill/unspill it). It's another reason I don't like to think of its "allocation" which sounds like a static thing. I'm unsure the names I'm suggesting are the best, but I think they are a mild improvement on what we have. At least, they helped make it clear to me where we have limitations in our current API.

vext01 commented 3 months ago

I'll merge this.

One of the challenges is that in a "real" register allocator, the location of a variable changes over time (as you spill/unspill it). It's another reason I don't like to think of its "allocation" which sounds like a static thing.

I dunno about that. When I am allocated a seat on a plane, I don't expect to sit there forever :)

ltratt commented 3 months ago

Looks like a transient PT error? Maybe retry?