cranelift/x64: implement cls for all integer types#12644
cranelift/x64: implement cls for all integer types#12644shreyasravi320 wants to merge 2 commits intobytecodealliance:mainfrom
Conversation
shreyasravi320
requested review from
alexcrichton
and removed request for
a team
github-actions
bot
added
cranelift
There was a problem hiding this comment.
For runnable tests we typically use the runtest directory instead of isa/x64. I see as well there's already a cls.clif file in that directory, so can that file be enabled for x64, and perhaps these tests merged there too?
There was a problem hiding this comment.
Got it. Added x86 support in the cls test there, and added the i128 tests. The existing tests were pretty comprehensive so I just added the i128 tests in the same format.
| (rule 1 (do_cls $I64 src) | ||
| (let ((shifted Gpr (x64_sarq_mi src 1)) | ||
| (xored Gpr (x64_xor $I64 src (RegMemImm.Reg shifted))) | ||
| (clz Gpr (do_clz $I64 $I64 xored))) | ||
| (x64_sub $I64 clz (RegMemImm.Imm 1)))) | ||
|
|
||
| (rule 0 (do_cls $I32 src) | ||
| (let ((shifted Gpr (x64_sarl_mi src 1)) | ||
| (xored Gpr (x64_xor $I32 src (RegMemImm.Reg shifted))) | ||
| (clz Gpr (do_clz $I32 $I32 xored))) | ||
| (x64_sub $I32 clz (RegMemImm.Imm 1)))) |
There was a problem hiding this comment.
These two rules can be merged together through the x64_sar helper perhaps?
There was a problem hiding this comment.
Sounds good. I decided to keep x64_sarq_mi in the cls rule for i128 since it would be more effort to switch that over to x64_sar.
2 participants
Fixes #5107
clsimplemented with the identitycls(x) = clz(x ^ (x >> 1)) - 1All integer types (
i8,i16,i32,i64, andi128) handledAdded tests to
filetests