New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/compile: move Type.Width and Type.Align into Extra fields #15308

Open
josharian opened this Issue Apr 14, 2016 · 5 comments

Comments

Projects
None yet
4 participants
@josharian
Contributor

josharian commented Apr 14, 2016

This is a placeholder issue for work I plan to do, probably not for Go 1.7 at this point. If anyone else wants to work on it, though, they are welcome to.

Only two gc.Types have potentially expensive to calculate width and alignments: structs and (maybe) arrays. We should move Type.Width and Type.Align into StructType and ArrayType, thus shrinking gc.Type for the vast majority of uses.

However, Width is currently overloaded to track some typechecking state. Plan is to change Deferwidth from a bool to a bitflag and start explicitly representing that state--has a width calculation been done? Is the width calculation deferred?

Also, most callsites refer to Width and Align directly. We'll need to switch them to use the Size method and a new Align method, remove unnecessary direct calls to dowidth, and teach the Size and Align methods the constant width and alignment for non-struct, non-array types.

@josharian josharian added this to the Unplanned milestone Apr 14, 2016

@randall77

This comment has been minimized.

Contributor

randall77 commented Apr 14, 2016

(the need to call) dowidth needs to die a horrible death.

@josharian

This comment has been minimized.

Contributor

josharian commented Apr 14, 2016

It might not be a horrible death, but I do fear that it'll be a slow one.

@josharian josharian added the ToolSpeed label Apr 16, 2016

@EliCDavis

This comment has been minimized.

EliCDavis commented Sep 23, 2018

Has this been resolved?

@josharian

This comment has been minimized.

Contributor

josharian commented Oct 9, 2018

@EliCDavis I don't think so. However, it is possible that @griesemer has plans to move the typechecker over to the new AST soon enough that this would not be worth doing.

@griesemer

This comment has been minimized.

Contributor

griesemer commented Oct 10, 2018

@EliCDavis Nope. There are other issues with dowith, though. I've started on moving type-checking over, but not much will be happening while we are not in the freeze.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment