On Sun, Nov 05, 2017 at 04:29:57PM +0800, Christopher Li wrote:
> >commit e4a0824120939235e40277a57425a72fbfcd5b9b
> >Author: Luc Van Oostenryck <luc.vanoostenryck@xxxxxxxxx>
> >Date: Sun Mar 26 19:35:14 2017 +0200
> >
>
> I think this patch ideally should be combined with the next patch.
> As it is, this patch is not self contain. There is no user of this table.
I'm fine with this. Certainly when:
*) the user is introduced in the next patch
*) there is 'static but unused' warnings issued
> ...
> > static int simplify_seteq_setne(struct instruction *insn, long long value)
> > {
> > pseudo_t old = insn->src1;
> >@@ -484,7 +460,7 @@ static int simplify_seteq_setne(struct instruction *insn, long long value)
> > // and similar for setne/eq ... 0/1
> > src1 = def->src1;
> > src2 = def->src2;
> >- 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.
> 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.
-- Luc
--
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
