Fix LLVM byte alignment for real #803
Conversation
| @@ -580,19 +580,22 @@ object Transformer { | |||
| } | |||
|
|
|||
| def pushFrameOnto(stack: Operand, environment: machine.Environment, returnAddressName: String, sharer: Operand, eraser: Operand)(using ModuleContext, FunctionContext, BlockContext) = { | |||
| if (!environment.isEmpty) { | |||
There was a problem hiding this comment.
Should we create an additional function for this? (like in popEnvironmentFrom)
|
After some more thought I'm not sure how my initial change would fix all of the alignment problems. Would my second solution in which we use packed structural types, also be okay (2-byte change)? In general not packing structs has all kinds of potential benefits for performance but they don't apply to us anyway, since such optimizations would segfault right now(?). Because our stack allocation is fundamentally based on knowing the exact allocation size, I believe this to be the only correct choice. |
marvinborner
changed the title
| @@ -428,7 +428,7 @@ object Transformer { | |||
| case machine.Type.Prompt() => 8 // TODO Make fat? | |||
| case machine.Type.Stack() => 8 // TODO Make fat? | |||
| case machine.Type.Int() => 8 // TODO Make fat? | |||
| case machine.Type.Byte() => 8 | |||
| case machine.Type.Byte() => 1 | |||
There was a problem hiding this comment.
Didn't @phischu just change this from 1 to 8 to fix something else?
There was a problem hiding this comment.
This was an insufficient fix as described in #785 (comment)
|
I'll interpret the thumbs up by @phischu as an approve and merge this :) |
Closes #794.
I've also added a test but it's a bit useless since the buggy parts all get optimized away and we currently only run tests with optimizations.