On 1/24/25 13:10, Michael Clark wrote:
+static x86_opc_prefix x86_table_make_prefix(const x86_opc_data *d,
+ const x86_opr_data *o, const x86_ord_data *p)
+{
+ x86_opc_prefix tp;
+ memset(&tp, 0, sizeof(tp));
+
+ /* extract prefix and synthesize width prefixes */
+ switch (x86_enc_type(d->enc)) {
+ case x86_enc_t_lex:
+ case x86_enc_t_vex:
+ case x86_enc_t_evex:
+ switch (d->enc & x86_enc_w_mask) {
+ case x86_enc_w_wig:
+ case x86_enc_w_wn:
+ case x86_enc_w_wb:
+ case x86_enc_w_w0: break;
+ case x86_enc_w_w1: tp.pfx = x86_enc_p_rexw; break;
+ case x86_enc_w_wx: tp.pfx_w = x86_enc_p_rexw; /* fallthrough */
+ case x86_enc_w_ww: tp.pfx_o = x86_enc_p_66; break;
+ }
+ break;
+ }
+
+ /* find register or memory operand mapping to modrm.rm field
+ * so that we can add mod=0b11 or mod!=0b11 to modrm mask */
+ tp.modfun = x86_enc_func(d->enc) == x86_enc_f_modrm_n;
+ for (size_t i = 0; i < array_size(o->opr) && o->opr[i]; i++) {
+ uint isreg = x86_opr_type_val(o->opr[i]) >= x86_opr_reg;
+ uint ismem = x86_opr_has_mem(o->opr[i]);
+ uint ismrm = x86_ord_type_val(p->ord[i]) == x86_ord_mrm;
+ if (ismrm) {
+ if (isreg && !ismem) {
+ tp.modreg = 1; /* mod == 0b11 */
+ break;
+ } else if (!isreg && ismem) {
+ tp.modmem = 1; /* mod != 0b11 */
+ break;
+ }
+ }
+ }
I am already relatively strictly following QEMU conventions for
single-line and multi-line comments. I don't remember seeing a
checkpatch.pl report for the above multi-line comment. I wonder
if the constraint is restricted to top-level comments? I left
this as-is due to symmetry with the other comments in this block
as they are single-line and it looks better. this is very pedantic.
so am curious about a determination on the commenting convention.
I have a disagreement about unbraced 'if' in switch because IMHO
the QEMU rules make combinatorial logic sparse and hard to read.
I also typically put the first brace of functions on their own
line so there is space between function signature and first line.
those are the two main rules I break. my coding-style has lots of
switch statements because I rely on switch constant analysis and
jump table conversion because it tends to translate to fast code.
also I linearize them explicitly because not all compilers detect
a constant stride for index canonicalization in switch constants.
that is where the bulk of the checkpatch.pl warnings show up.
+ /* explict second opcode byte has mod == 0b11 */
+ if (d->opm[1] == 0xff && (d->opc[1] & 0xc0) == 0xc0 &&
+ !tp.modreg && !tp.modmem)
+ {
+ tp.modreg = 1;
+ }
+
+ return tp;
+}
fyi I have made a x86-mini-v2 branch with the changes I
mentioned in an email yesterday but it is still in flux:
https://github.com/michaeljclark/qemu/commits/x86-mini-v2
I still don't expect this code to be merged any-time-soon.
maybe in the future if QEMU gains the ability to run TCG
full-system under hypervisor with accelerated page tables.
I'm also considering digging out the most recent QEMU or tcc
TCG that is fully MIT-licensed as a starting point for IR.
the patch set may eventually become compelling if there is
a complete TCG supporting all AVX-512 vector ops. maybe this
is already there but I am not reading the GPL code in QEMU.
I am unsure about that code because a lot of code is by-file
licensed. it is questionable when the provenance originates
inside QEMU as opposed to MIT-licensed or dual-licensed code
(like FreeType) that is clearly of external origin. as a
modern embedded TCG would be quite useful in Rui's chibicc
or the MIT-licensed version of tcc:
- https://github.com/rui314/chibicc
- https://github.com/absop/Tinycc/blob/master/RELICENSING
and I am confessing about a snippet of code. this may look
familiar. there is one enum and two functions that were
derived from TCG X86 but they are unused. so I should add
Fabrice's original TCG MIT-license header or remove them.
I am pretty sure I checked the provenance for the version
of TCG that I used but I need to add an explicit reference
or delete these definitions. they can't really be written
a different way because they are more like reference data
and invariant logic based on X86 ISA constraints:
+/*
+ * test conditions
+ */
+
+enum
+{
+ /* non-signed */
+ x86_never = (0 | 0 | 0 | 0),
+ x86_always = (0 | 0 | 0 | 1),
+ x86_eq = (8 | 0 | 0 | 0),
+ x86_ne = (8 | 0 | 0 | 1),
+ /* signed */
+ x86_lt = (0 | 0 | 2 | 0),
+ x86_ge = (0 | 0 | 2 | 1),
+ x86_le = (8 | 0 | 2 | 0),
+ x86_gt = (8 | 0 | 2 | 1),
+ /* unsigned */
+ x86_ltu = (0 | 4 | 0 | 0),
+ x86_geu = (0 | 4 | 0 | 1),
+ x86_leu = (8 | 4 | 0 | 0),
+ x86_gtu = (8 | 4 | 0 | 1),
+};
+/*
+ * invert condition
+ */
+
+static inline uint x86_invert_cond(uint c) {
+ return c ^ 1;
+}
+
+/*
+ * swap condition operands
+ */
+
+static inline uint x86_swap_cond(uint c) {
+ return c & 6 ? c ^ 9 : c;
+}
