On Thu, Sep 24, 2026 at 08:17:17AM -0700, Richard Henderson wrote: > Date: Thu, 24 Sep 2026 08:17:17 -0700 > From: Richard Henderson <[email protected]> > Subject: Re: [PATCH 13/17] target/i386/tcg: add decode functionality for APX > > On 9/23/26 10:30, Paolo Bonzini wrote: > > + case X86_EVEX_APX: > > + case X86_EVEX_APX_cmp: > > + case X86_EVEX_APX_pp2: > > + case X86_EVEX_APX_zu: > > + /* Only reached through EVEX map 4. */ > > + assert(!(s->prefix & PREFIX_VEX)); > > + if (s->vex_l) { > > + goto illegal; > > + } > > + if ((s->prefix & PREFIX_EVEX) && !validate_evex_apx(s, decode)) { > > + return false; > > + } > > They're only reached by map4, so surely > > assert(s->prefix & PREFIX_EVEX); > > is more appropriate, and the subsequent test for evex moot.
I think the code comment is misleading (and assert seems right). Not only evex map4, but also some legacy decoding reaches here, and those legacy decoding share the decode helpers with evex map4, e.g. decode_group1/decode_group3/decode_group4_5. And REX2 could also reach here since it reuses legacy decoding table in decode_REX2_map0. Maybe we can refine the commnet like: /* EVEX map4/REX2/legacy all may reach here. Only VEX won't */ or we can drop that comment directly? :-) Regards, Zhao
