Implement stuff required to elide instruction loading in yklua. #1642
Conversation
This includes: - Promote support for pointers. - Implementing various todo! relating to constant pointers. - Optimisations for ptr_add upon const pointers. - SpillState support for const pointers.
|
@ltratt you could help me with a test for the I had a brief look, but I don't know this part of the system at all. |
| // unwrap cannot fail since pointers are sized. | ||
| let bitw = self.m.type_(self.m.ptr_tyidx()).bitw().unwrap(); | ||
|
|
||
| match bitw { |
There was a problem hiding this comment.
This is an assert masquerading as a todo I think?
There was a problem hiding this comment.
Yeah, just the result of copying code from elsewhere :) Will fix.
| self.ra.assign_const_int(iidx, bits, v); | ||
| } | ||
| VarLocation::ConstPtr(v) => { | ||
| // unwrap can't fail because pointers are sized. |
| // the "pointer index type" before computing the offset. | ||
| let cidx = self | ||
| .m | ||
| .insert_const(Const::Ptr((*cptr as isize + off as isize) as usize))?; |
There was a problem hiding this comment.
We avoid as if at all possible. These should probably be isize::try_from(...).unwrap() (etc.) and you will need to decide if you want saturating_add or wrapped_add etc. to deal with poison cases. + will bork in debug mode when it detects overflow, which is not what we want, particularly as these situations do happen because select later gets rid of the poisoned value.
There was a problem hiding this comment.
By as casting the offset to isize we truncate or sign extend, as is (I believe) required by LLVM.
This section of the llvm manual:
The indices are first converted to offsets in the pointer’s index type. If the currently indexed type is a struct type, the struct offset corresponding to the index is sign-extended or truncated to the pointer index type. Otherwise, the index itself is sign-extended or truncated, and then multiplied by the type allocation size (that is, the size rounded up to the ABI alignment) of the currently indexed type.
I must admit, I find this part of the documentation confusing. I hope I've interpreted it correctly.
There was a problem hiding this comment.
The problem with as is that if you change the types later, it still "works" but does weird things. It's better to use the full type names, as we do elsewhere. Only if you really have to should you use as (regrettably sometimes you do have to; I don't think this is one of those cases, but I might be wrong).
There was a problem hiding this comment.
I think we can split this into two cases: truncate and sign-extend.
todo! the former, since I don't anticipate seeing that any time soon. isize::try_from() the latter.
There was a problem hiding this comment.
BTW, in terms of the + stuff, the BinOp::Add optimisation probably serves as a useful template for this.
| @@ -1762,6 +1776,12 @@ mod test { | |||
|
|
|||
| #[test] | |||
| fn opt_ptradd() { | |||
| // FIXME: The offset operand of ptr_add should allow specifying a type (e.g. `ptr_add %0, | |||
There was a problem hiding this comment.
This will need to be fixed in this PR. We need to make the optimiser tests as complete as possible, because reasoning about it is already beyond my pea brain.
| @@ -1795,6 +1815,24 @@ mod test { | |||
| black_box %0 | |||
| ", | |||
| ); | |||
|
|
|||
| // constant pointer optimisations. | |||
There was a problem hiding this comment.
This needs to test all the edge cases too e.g. adding to the max value etc.
|
This looks good! I do have several comments, mostly because the optimiser has to be kept in tip-top condition. But this is already a solid start! |
This includes: