On 8/9/19 9:12 PM, Jan Bobek wrote: > This is a v2 of the patch series posted in [1]. Patches 1-9 are just > cleanups; patches 10-39 are something actually interesting. Compared > to v1, I started using preprocessor more extensively to generate > repetitive boilerplate code; opinions/alternatives are welcome and > appreciated.
This is tricky. I'm not keen on code entirely expanded via macros because it becomes extremely difficult to debug. All statements get recorded at the same line of the location of the expansion, which makes the gdb "step" command finish the entire function because there is no next line. Once upon a time I wrote some code that's extremely macro crazy: https://sourceware.org/git/?p=glibc.git;a=blob_plain;f=soft-fp/op-common.h;hb=HEAD It has been extremely difficult to maintain over the years. We have just recently gotten rid of some of the macros in the softmmu code https://patchwork.ozlabs.org/project/qemu-devel/list/?series=105441 replacing most of them with inline functions. A lot of what you have needs very little adjustment to address the debugging problem. E.g. > +#define INSNOP_INIT(opT, init_stmt) \ > + static int insnop_init(opT)(CPUX86State *env, DisasContext *s, \ > + int modrm, insnop_t(opT) *op) \ > + { \ > + init_stmt; \ > + } .... > +INSNOP( > + M, TCGv, > + do { > + if (decode_modrm_mod(env, s, modrm) == 3) { > + INSNOP_INIT_FAIL; > + } else { > + INSNOP_INIT_OK(s->A0); > + } > + } while (0), > + do { > + assert(*op == s->A0); > + gen_lea_modrm(env, s, modrm); > + } while (0), > + INSNOP_FINALIZE_NOOP) Rearrange this as #define INSNOP_INIT(OPT) \ static bool insnop_##OPT##_init(CPUX86State *env, DisasContext *s, \ int modrm, insnop_##OPT##_t *op) #define INSNOP_PREPARE(OPT) \ static void insnop_##OPT##_prepare(CPUX86State *env, DisasContext *s, \ int modrm, insnop_##OPT##_t *op) INSNOP_INIT(M) { if (decode_modrm_mod(env, s, modrm) == 3) { INSNOP_INIT_FAIL; } else { INSNOP_INIT_OK(s->A0); } } INSNOP_PREPARE(M) { assert(*op == s->A0); gen_lea_modrm(env, s, modrm); } etc and suddenly the entire expansion does not occur on a single line. Further specific commentary to follow. r~
