Thanks, Jakob! That's the sort of feedback I was looking for. On Wednesday, November 13, 2019 at 2:57:33 AM UTC-8, Jakob Kummerow wrote: > > Thanks for reporting your observation, and for asking this question! > > I'm not familiar with this code, but given that there are no x64 CPUs in > existence or on the horizon that would have more GPRs than XMM registers, > my gut feeling is that it's fine to keep this code as-is. At best, I would > add a STATIC_ASSERT(XMMRegister::kNumRegisters >= Register::kNumRegisters) > here, but I'm not sure that'd be worth doing, because it seems exceedingly > unlikely that a new CPU generation for an existing architecture would add > more general-purpose registers. AFAIK that has never happened in the past > either, because it is supremely difficult to pull off: all software would > have to be recompiled in order to take advantage of these registers, and > once it is recompiled in that way, the binaries won't run on older > hardware. It would probably also require changes to the binary encoding of > machine code, just to be able to address/specify these additional > registers. In short, if a manufacturer wanted to introduce a CPU with more > general-purpose registers, then that wouldn't be x86_64 any more -- it > would be "x86_64_v2" or whatever, and V8 and other compilers would probably > have to make significant changes to support it. > > > On Tue, Nov 12, 2019 at 11:13 PM <[email protected] <javascript:>> > wrote: > >> I'm relatively new to the V8 code base, but I recently noticed that the >> x64 code-generation logic for instructions which involve both vector (XMM) >> and integer (GPR) registers seems to have a flaw -- although the code works >> b/c the number of integer GPRs and XMM registers (in AVX2-class machines) >> are less-than or equal at 16; and for an AVX3/AVX-512 machine there will be >> a difference (16 vs. 32), but the number of XMM registers is still larger >> than the number of integer GPRs. >> >> The issue occurs in-and-around callers of vinstr(), an example of which >> is vcvtlsi2sd() >> >> void vcvtlsi2sd(XMMRegister dst, XMMRegister src1, Register src2) { >> XMMRegister isrc2 = XMMRegister::from_code(src2.code()); >> vinstr(0x2a, dst, src1, isrc2, kF2, k0F, kW0); >> } >> >> >> Here, the integer GPR register argument (src2) is treated as an XMM >> register when calling from_code(). This means that the incoming register >> code is "subject to" the limits of the XMMRegister incarnation of >> RegisterBase. >> >> i.e. it's subject to the kDoubleAfterLast limit, instead of the >> kRegAfterLast limit. >> >> >> If the XMMRegister space is equal to or larger than the integer GPR >> space, then this "works" (b/c the check is a simple less-than) -- but if >> there is ever an inversion (where the number of GPRs exceeds the number of >> XMM registers), then the code will fail. >> >> This isn't a functional bug right now, but it seems like the logic which >> is coercing a Register to XMMRegister is incorrect. Is this worth a fix? >> It affects downstream code of course, as the type signatures of functions >> called from here "down" all assume XMMRegisters as arguments (for the most >> part). >> >> >> -- >> -- >> v8-dev mailing list >> [email protected] <javascript:> >> http://groups.google.com/group/v8-dev >> --- >> You received this message because you are subscribed to the Google Groups >> "v8-dev" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected] <javascript:>. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/v8-dev/31f4aca7-f518-4572-b88b-031b76827db9%40googlegroups.com >> >> <https://groups.google.com/d/msgid/v8-dev/31f4aca7-f518-4572-b88b-031b76827db9%40googlegroups.com?utm_medium=email&utm_source=footer> >> . >> >
-- -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/8a517b0a-bec1-4316-872b-f88d070013a4%40googlegroups.com.
