size_of_val, min_align_of_val implementation#1092
Conversation
40 new passing tests (this PR and PR #1091: static refs)
mariaKt
force-pushed
the
mk/intrinsics-impl
branch
from
cf7ad67 to
ddd1c22
Compare
dkcumming
left a comment
dkcumming
left a comment
There was a problem hiding this comment.
Nice! I left a comment about the sentinel value that I think we should consider. But approving as it should not block this.
| <types> TYPESMAP </types> | ||
| requires isTy(#extractOperandType(TYPESMAP, ARG, LOCALS)) | ||
| andBool isTy(pointeeTy(lookupTy(TYPESMAP, {#extractOperandType(TYPESMAP, ARG, LOCALS)}:>Ty))) | ||
| andBool #metadataSize(TYPESMAP, pointeeTy(lookupTy(TYPESMAP, {#extractOperandType(TYPESMAP, ARG, LOCALS)}:>Ty))) =/=K dynamicSize(1) |
There was a problem hiding this comment.
At first I thought this was too permissive as it only restricts dynamicSize(1) for all slices. However, looking at rt/types.md the codomain of #metadataSize is { staticSize(N), dynamicSize(1), noMetadataSize }. So in this case, dynamicSize(1) is a sentinel value only.
Not sure if it is worth changing but it would be nice to continue the pattern of using sentinels that are logically distinct from sensible values. I think dynamicSize(-1) would be better here because if one knows that it is impossible immediately by reading it so either there is an error or it is a sentinel only.
2 participants
This PR implements
size_of_val,min_align_of_valfor sized types.I show the stuck state and passing state before and after each one's implementation, and remove the output files at the end (included only to help review).
We do not handle unsized types atm, left for future work (their dynamic size calculation is likely to change).
Finally, we update the skip lists for the LLVM and Haskell backend. Note that these newly passing tests reflect not just this PR, but also PR #1091.