On Wed, 2021-03-24 at 07:44 -0600, Richard Henderson wrote:
> On 3/24/21 2:00 AM, Robert Hoo wrote:
> > + if ((env->xcr0 & XFEATURE_AVX512) == XFEATURE_AVX512) {
> > + /* XSAVE enabled AVX512 */
> > + nb = (env->hflags & HF_CS64_MASK) ? 32 : 8;
> > + for (i = 0; i < nb; i++) {
> > + qemu_fprintf(f, "ZMM%02d=0x%016lx %016lx %016lx
> > %016lx %016lx "
> > + "%016lx %016lx %016lx\n",
> > + i,
> > + env->xmm_regs[i].ZMM_Q(7),
> > + env->xmm_regs[i].ZMM_Q(6),
> > + env->xmm_regs[i].ZMM_Q(5),
> > + env->xmm_regs[i].ZMM_Q(4),
> > + env->xmm_regs[i].ZMM_Q(3),
> > + env->xmm_regs[i].ZMM_Q(2),
> > + env->xmm_regs[i].ZMM_Q(1),
> > + env->xmm_regs[i].ZMM_Q(0));
> > + }
>
> Dump opmask regs?
OK
>
> > + } else if (env->xcr0 & XFEATURE_AVX) {
>
> This is normally a 2-bit test.
I beg your pardon. What 2 bits?
>
> > + /* XSAVE enabled AVX */
> > + nb = env->hflags & HF_CS64_MASK ? 16 : 8;
> > + for (i = 0; i < nb; i++) {
> > + qemu_fprintf(f, "YMM%02d=0x%016lx %016lx %016lx
> > %016lx\n",
> > + i,
> > + env->xmm_regs[i].ZMM_Q(3),
> > + env->xmm_regs[i].ZMM_Q(2),
> > + env->xmm_regs[i].ZMM_Q(1),
> > + env->xmm_regs[i].ZMM_Q(0));
> > + }
> > + } else { /* SSE and below cases */
> > + nb = env->hflags & HF_CS64_MASK ? 16 : 8;
> > + for (i = 0; i < nb; i++) {
> > + qemu_fprintf(f, "XMM%02d=0x%016lx %016lx",
> > + i,
> > + env->xmm_regs[i].ZMM_Q(1),
> > + env->xmm_regs[i].ZMM_Q(0));
> > + if ((i & 1) == 1)
> > + qemu_fprintf(f, "\n");
> > + else
> > + qemu_fprintf(f, " ");
>
> I'd be tempted to merge that second printf into the first, with "%s"
> and (i & 1
> ? "\n" : " "). Otherwise you'll need to add braces to that IF to
> satisfy
> checkpatch.
Sure. I just retained previous code.
BTW, checkpatch didn't warn me on this. It escaped.:)
>
> > +#define XFEATURE_X87 (1UL << 0)
> > +#define XFEATURE_SSE (1UL << 1)
> > +#define XFEATURE_AVX (1UL << 2)
> > +#define XFEATURE_AVX512_OPMASK (1UL << 5)
> > +#define XFEATURE_AVX512_ZMM_Hi256 (1UL << 6)
> > +#define XFEATURE_AVX512_Hi16_ZMM (1UL << 7)
> > +#define XFEATURE_AVX512 (XFEATURE_AVX512_OPMASK | \
> > + XFEATURE_AVX512_ZMM_Hi256 | \
> > + XFEATURE_AVX512_Hi16_ZMM)
>
> Except for the last, these already exist under the name
> XSTATE_*_MASK.
Ah, my poor eye sight. They even exist in the same file. Thanks
pointing out.
>
> I think you can just as well declare local variables to hold the 3
> bits for the
> avx512 test and the 2 bits for the avx test.
>
Sure.
>
> r~
