Signed division via Arith #54
Conversation
|
Due to discussion in zulip I think we can don't care about overflows in this PR and make another PR which cares about all overflows work properly after most part of arithmetic will be implemented (and maybe zk processor switches to 64 bit?) |
| collector.reg_clobbers(clobbered); | ||
| // Division via ARITH perform different operations with registers A, B, C, D, | ||
| // so they can not be result register for proper division | ||
| collector.reg_fixed_def(rd, e0()); |
There was a problem hiding this comment.
Huh, I was under impression that in order to check DIV with ARITH, we’d have to use E, D and B registers as inputs, and A would be the output or something like that? I’m quite lost how this is supposed to work at this point…
There was a problem hiding this comment.
Hmm, yes, with convention that div performs A := E / B we have 2 lines of bytecode less. But we still have to fix all the registers, because if we will allow register allocator freely choose output register, B for example can be chosen, and it will broke the ARITH assertation. Done here
There was a problem hiding this comment.
D and C also should be set to zero when we perform ARITH assertation
There was a problem hiding this comment.
So I think A := E / B is optimal for DivArith in assumption less lines of zkAsm code => better performance
| ; ((_ InstOutput (gen_div_overflow x y $I64)) | ||
| ; (_ InstOutput (gen_div_by_zero y)) ) | ||
| ; (rv_div x y))) | ||
| (rule -1 (lower (has_type (fits_in_32 ty) (sdiv x y))) |
There was a problem hiding this comment.
You can drop this rule entirely I believe, and have something like
(rule (lower (sdiv x y))
(zk_div x y))
There was a problem hiding this comment.
Yes, it works, added here. But maybe splitting to two lowering patterns will help in future when we will adding smaller types?
There was a problem hiding this comment.
I don’t think it is worth thinking about it right now – ISLE is reasonably easy to extend when needed, but quite hard to simplify later in my experience, so its better to do the simple thing first. In fact I hope that we can remove most of the isle code and replace it with the few simple rules like these initially.
Signed division via arith for i32 and i64