s390x: simplify ceil/floor code#246
s390x: simplify ceil/floor code#246neuschaefer wants to merge 1 commit intoopenresty:v2.1-agentzhfrom
Conversation
The previous change[1] to this code left quite a bit of dead code behind, when it switched vm_round from using DIDBR (divide to integer) to FIDBR (load floating-point integer). Simplify the code by removing the remains of the previous implementation. As a side effect, math.ceil() etc. now execute without crashing in QEMU, which was previously not the case because QEMU doesn't implement DIDBR. [1]: openresty#177
|
Perhaps it's an overkill, but I'm adding DIVIDE TO INTEGER to QEMU to resolve this: https://lore.kernel.org/qemu-devel/20260129190902.196262-1-iii@linux.ibm.com/ |
|
@iii-i I think both changes are good to have, independently of each other |
|
@neuschaefer Should I merge this PR? |
Yes, I think it's a worthwhile improvement regardless of changes in QEMU |
|
FWIW this change looks good to me. I looked up some history; initially it was implemented in linux-on-ibm-z/LuaJIT@e0e98f9 with the comment "x86_64 does floating point mod", but I don't think this is true (anymore): they seem to be using floating point bit tricks now: https://github.com/LuaJIT/LuaJIT/blob/707c12bf00dafdfd3899b1a6c36435dbbf6c7022/src/vm_x64.dasc#L2550. We can add something like this some day, but for now this simplification looks like strictly an improvement. |
3 participants
The previous change1 to this code left quite a bit of dead code behind, when it switched vm_round from using DIDBR (divide to integer) to FIDBR (load floating-point integer). Simplify the code by removing the remains of the previous implementation.
As a side effect, math.ceil() etc. now execute without crashing in QEMU, which was previously not the case because QEMU doesn't implement DIDBR.