Feature/int widths #96
Conversation
TODO: check how megaparsec works with this because it appears very incorrect
| @@ -126,8 +126,8 @@ codeGenBlock_ = fmap snd . codeGenBlock | |||
| codeGenSimpleType :: SimpleType -> CG IR.Reg | |||
| codeGenSimpleType = \case | |||
| T_Unit -> newRegWithSimpleType (-1) | |||
| T_Int64 -> newRegWithSimpleType (-2) | |||
| T_Word64 -> newRegWithSimpleType (-3) | |||
| -- T_Int width -> newRegWithSimpleType (-2) | |||
There was a problem hiding this comment.
@csabahruska do you have an existing plan how to implement this in these abstract interpretations?
There was a problem hiding this comment.
I was wondering a bit while I was writing this why a mapping from types to registers wasn't handled by Data.Map or something similar. Maybe assigning a unique type to the next available register would work?
| = T_Int64 | ||
| | T_Word64 | ||
| = T_Int Word32 | ||
| | T_Word Word32 |
There was a problem hiding this comment.
I would use Word32 as a parameter to define the concrete width of these types. Some ADT which is better aligned with the LLVM types would be better.
There was a problem hiding this comment.
I originally used Word but llvm-hs-pure uses Word32; I got compile errors trying to use Word because of that. However, the maximum width in the LLVM spec is (2^24) - 1 bits.
There was a problem hiding this comment.
Ah got it, if LLVM-hs uses in the API than no objections from my side anymore :)
|
It is a good practice to write tests for the changed code paths. |
|
What is your objective? Maybe we can lay out an implementation path that makes your experiment easier. |
|
My objective here is to be able to have more number types than RE: Tests, as far as I know I need to get it compiling first before I can start testing. The current commit does not compile. |
|
FFI sounds great! Do you see a way how HPT analysis will be able to handle these additions? |
|
What do you mean by "handle"? What would be different between |
| _ -> error $ "invalid arguments: " ++ show params ++ " " ++ show args ++ " for " ++ unpackName name | ||
|
|
||
| int_str = case args of | ||
| [RT_Lit (LInt64 a)] -> string $ fromString $ show a | ||
| [RT_Lit (LInt 64 a)] -> string $ fromString $ show a |
There was a problem hiding this comment.
Is it intended to use a specific width in each of these functions?
There was a problem hiding this comment.
So far, I have converted primops from T_Int64 to T_Int 64, but the end goal is to generalize them (part of checkbox three, I should probably specify that). Note that operations will only be defined for numbers of the same width, i.e. add :: i8 -> i7 -> i9 should be invalid (same as how LLVM does it).
|
#32 Looks awesome! I'll be sure to [merge/rebase, whichever one it is] my fork once its merged. I definitely agree with merging after it, because this being merged first would probably make your work harder, and that being merged first will most likely make my work easier. |
|
The code rewriting is almost done for #32. What is left is to merge the changes, and address any concerns. Feel free to review the PRs :) |
TODO:
123u8,1i12345)Opening this as a draft PR. It's my first time ever working on a major Haskell project, so thorough reviews would be very very appreciated 😄