On Mon, Nov 6, 2017 at 1:10 AM, Luc Van Oostenryck <luc.vanoostenryck@xxxxxxxxx> wrote: >> >- insn->opcode = compare_opcode(opcode, inverse); >> >+ insn->opcode = inverse ? opcode_table[opcode].negate : opcode; >> >> I think it would be better to have some kind of assert check here, the opcode >> you swap from the table is indeep opcode. Because you assign the opcode >> array using sparse index. It is easy to miss a spot creating the empty slot in >> the table. > > Sorry, I see the words, I sorta gues what you mean but I can't parse > what you wrote. Let me re phrase. The opcode_table has a lot of zero in them. Even though you thing you give all the op code worthwhile to have .negate an opcode. The rest of op has .negate to zero. My suggestion is that make a assert check this new opcode you get back from the table is not zero. You initialize the op code table using position index. It is possible to missing a slot. So have an assert check here is good. > >> Also, the opcode table might be able to compressed only contain >> section of the BINCMP opcodes. > > Yes, it's something that may be done after all the related changes > are done if the speedup is worth the added complexity. This series will first go into a topic branch any way. Clearly your opcode is only apply to OP_SETXXXX friends. I more compact data section is good. Chris -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
