Alignment overhaul#13
Conversation
This makes it easier for frontends to give it the correct alignment.
Determines the default alignment similar to QBE itself
| assert!( | ||
| *align == 4 || *align == 8 || *align == 16, | ||
| "Invalid stack alignment" | ||
| ); | ||
| write!(f, "alloc{} {}", align, size) |
There was a problem hiding this comment.
Not really a fan of this, as it moves the error from compile-time to run-time, which we should avoid.
There was a problem hiding this comment.
Not sure, we have to provide some way to get the corresponding allocation instruction by giving an alignment from Type.align() or similar
There was a problem hiding this comment.
But what keeps you from writing qbe::Alloc(5, 0), which would result in a runtime error?
There was a problem hiding this comment.
An alternative would be to have Alloc(Type, Align) but that diverges too much from the original QBE IL. Let's just keep this as is.🙂
| Self::Aggregate(td) => match td.align { | ||
| Some(align) => align, | ||
| None => { | ||
| assert!(!td.items.is_empty(), "Invalid empty TypeDef"); |
There was a problem hiding this comment.
Is this really needed? Are typedefs required to have at least one item?
There was a problem hiding this comment.
Yes, absolutely. The only possible way to get empty typedef is not-yet-supported opaque typedefs, but they require explicit alignment.
2 participants
Description
Fixes issues related to field alignment in
Type.size()and addsType.align()helperToDo