On Fri, Mar 24, 2017 at 1:11 AM, Luc Van Oostenryck
<luc.vanoostenryck@xxxxxxxxx> wrote:
>
> Yes, indeed.
> In fact, I think it need a name which explain even more
> the purpose. I'll see.
Sure, I am open to suggestion.
BTW, the previous function is call compare_opcode() what
it actually do is invert_compare(). I would just move the inverse
test outside of this function and call it invert_compare() or
invert_compare_opcode()
> Yes, it's kinda ugly.
> OTOH, given that jumps are cheaper and cheaper on modern CPU
> and memory accesses slower and slower, is what you propose
> really an optimization? And even if it would, does it matter?
Well, jump through a indirect table is quit different than jump
to a fix offset. I don't know what modern CPU does now days. It
used to be indirect jump will flush the instruction pipe line.
>
> My preference, if performance mattered here, would be to play
> with the bits of the opcode, something like:
> #define OP_SET__EQUAL (1 << ...)
> #define OP_SET__SIGNED (1 << ...)
> #define OP_SET__LTHAN ...
> #define OP_SET__GTHAN ...
>
> ...
>
> int swap_compare_opcode(opcode) { return opcode ^= (OP_SET__LTHAN|OP_SET__GTHAN); }
That is ugly. I don't want that.
I cook up an untested patch include here to show what I have in mind.
It should be very close to your case statement way of writing it. You are
welcome to change it.
Chris
diff --git a/simplify.c b/simplify.c
index 3f39819..6e113d9 100644
--- a/simplify.c
+++ b/simplify.c
@@ -438,22 +438,24 @@ static int compare_opcode(int opcode, int inverse)
static int compare_swap(int opcode)
{
- switch (opcode) {
- case OP_SET_EQ: return OP_SET_EQ;
- case OP_SET_NE: return OP_SET_NE;
-
- case OP_SET_LT: return OP_SET_GT;
- case OP_SET_LE: return OP_SET_GE;
- case OP_SET_GT: return OP_SET_LT;
- case OP_SET_GE: return OP_SET_LE;
-
- case OP_SET_A: return OP_SET_B;
- case OP_SET_AE: return OP_SET_BE;
- case OP_SET_B: return OP_SET_A;
- case OP_SET_BE: return OP_SET_AE;
- default:
- return opcode;
- }
+
+#define CMP_OFFSET(opcode) ((opcode) - OP_BINCMP)
+ static const int swap_opcodes[CMP_OFFSET(OP_BINCMP_END) + 1] = {
+ [CMP_OFFSET(OP_SET_EQ)] = OP_SET_EQ,
+ [CMP_OFFSET(OP_SET_NE)] = OP_SET_NE,
+
+ [CMP_OFFSET(OP_SET_LT)] = OP_SET_GT,
+ [CMP_OFFSET(OP_SET_GT)] = OP_SET_LT,
+ [CMP_OFFSET(OP_SET_LE)] = OP_SET_GE,
+ [CMP_OFFSET(OP_SET_GE)] = OP_SET_LE,
+
+ [CMP_OFFSET(OP_SET_A)] = OP_SET_B,
+ [CMP_OFFSET(OP_SET_B)] = OP_SET_A,
+ [CMP_OFFSET(OP_SET_AE)] = OP_SET_BE,
+ [CMP_OFFSET(OP_SET_BE)] = OP_SET_AE,
+ };
+ assert(opcode >= OP_BINCMP && opcode <= OP_BINCMP_END);
+ return swap_opcodes[CMP_OFFSET(opcode)];
}
static int simplify_seteq_setne(struct instruction *insn, long long value)
