From: Paul K. <pv...@pv...> - 2013-04-30 20:17:10
|
stassats wrote: > +(define-vop (fixnum-len) [...] > + (:args (arg :scs (any-reg) :target res)) > + (:arg-types tagged-num) > + (:results (res :scs (unsigned-reg))) > + (:result-types unsigned-num) > + (:generator 25 > + (move res arg) > + (when (> n-fixnum-tag-bits 1) > + (inst shr res (1- n-fixnum-tag-bits))) Shouldn't that be an arithmetic shift? > + (if (sc-is res unsigned-reg) > + (inst test res res) > + (inst cmp res 0)) > + (inst jmp :ge POS) What's the SC test for? > + (inst not res) That's probably more quickly computed as: mov mask, res sar mask, (1- n-word-bits) xor res, mask Paul Khuong |
From: Stas B. <sta...@gm...> - 2013-05-01 08:46:21
|
Paul Khuong <pv...@pv...> writes: > stassats wrote: >> +(define-vop (fixnum-len) > [...] >> + (:args (arg :scs (any-reg) :target res)) >> + (:arg-types tagged-num) >> + (:results (res :scs (unsigned-reg))) >> + (:result-types unsigned-num) >> + (:generator 25 >> + (move res arg) >> + (when (> n-fixnum-tag-bits 1) >> + (inst shr res (1- n-fixnum-tag-bits))) > Shouldn't that be an arithmetic shift? Right, I should get a build where n-fixnum-tag-bits > 1. >> + (if (sc-is res unsigned-reg) >> + (inst test res res) >> + (inst cmp res 0)) >> + (inst jmp :ge POS) > What's the SC test for? That was copied from the original instruction. >> + (inst not res) > That's probably more quickly computed as: > > mov mask, res > sar mask, (1- n-word-bits) > xor res, mask That uses an additional register, is 2 bytes larger and is only slightly faster in my tests, not worth it. -- With best regards, Stas. |
From: <lut...@fr...> - 2013-05-01 11:33:50
|
Hi, Stas wrote: > Paul Khuong <pv...@pv...> writes: > > > stassats wrote: [...] > >> + (if (sc-is res unsigned-reg) > >> + (inst test res res) > >> + (inst cmp res 0)) > >> + (inst jmp :ge POS) > > What's the SC test for? > That was copied from the original instruction. this kind of SC test is quite common: to test if a value is 0, if it is in a register, "test" can be used, which has a shorter encoding than "cmp". "cmp" on the other hand can take one operand in memory, too. Just grep for "smaller instruction" in src/compiler/x86-64/*.lisp. It seemed so far not important or advantageous to me (as a person who added a few of these) to abstract that pattern into a macro or function. Regards, Lutz |
From: Paul K. <pv...@pv...> - 2013-05-01 11:53:05
|
Lutz Euler wrote: > Stas wrote: > >> Paul Khuong<pv...@pv...> writes: >> >>> stassats wrote: > [...] >>>> + (if (sc-is res unsigned-reg) >>>> + (inst test res res) >>>> + (inst cmp res 0)) >>>> + (inst jmp :ge POS) >>> What's the SC test for? >> That was copied from the original instruction. > > this kind of SC test is quite common: to test if a value is 0, if it is > in a register, "test" can be used, which has a shorter encoding than > "cmp". "cmp" on the other hand can take one operand in memory, too. Right, but the VOP wants an unsigned-reg for res (without any load-if condition). Paul Khuong |
From: <lut...@fr...> - 2013-05-01 12:46:58
|
Paul Khuong wrote: > Lutz Euler wrote: > > Stas wrote: > > > >> Paul Khuong<pv...@pv...> writes: > >> > >>> stassats wrote: > > [...] > >>>> + (if (sc-is res unsigned-reg) > >>>> + (inst test res res) > >>>> + (inst cmp res 0)) > >>>> + (inst jmp :ge POS) > >>> What's the SC test for? > >> That was copied from the original instruction. > > > > this kind of SC test is quite common: to test if a value is 0, if it is > > in a register, "test" can be used, which has a shorter encoding than > > "cmp". "cmp" on the other hand can take one operand in memory, too. > > Right, but the VOP wants an unsigned-reg for res (without any load-if > condition). You are right. Sorry, I didn't expect anything wrong in that direction so didn't look hard enough. The SC-test is unnecessary; one can always use the TEST instruction. The original VOP SIGNED-BYTE-64-LEN has the same unnecessary test. The change from "always use cmp" to "test the sc" there was commit 16028d14234d2acd0e6a3145a7364f2a52eabf63, which affects x86, too. Greetings, Lutz |