| Message ID | 20170324231421.14869-5-luc.vanoostenryck@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Fri, Mar 24, 2017 at 4:14 PM, Luc Van Oostenryck <luc.vanoostenryck@gmail.com> wrote: > More precisely, use a table to get the opcdoe corresponding > to the negated compare and use a more explicit name for the > function. Side note: this code should verify that it doesn't operate on a floating point compare. You can't negate a FP compare, because the negation doesn't necessarily have the opposite value. For example, "a < b" is *not* the same as "!(a >= b)" for floating point values when one of them is a NaN. Both < and >= will compare as false, so "negating" the op won't actually negate the resulting logical operation. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Mar 25, 2017 at 12:24 AM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Fri, Mar 24, 2017 at 4:14 PM, Luc Van Oostenryck > <luc.vanoostenryck@gmail.com> wrote: >> More precisely, use a table to get the opcdoe corresponding >> to the negated compare and use a more explicit name for the >> function. > > Side note: this code should verify that it doesn't operate on a > floating point compare. > > You can't negate a FP compare, because the negation doesn't > necessarily have the opposite value. > > For example, "a < b" is *not* the same as "!(a >= b)" for floating > point values when one of them is a NaN. Both < and >= will compare as > false, so "negating" the op won't actually negate the resulting > logical operation. > > Linus Yes, indeed. I've some plan to add better handling of floating-point and the compare is part of it. It'll need a new set of instructions to do it correctly (precisely because for fp numbers once you care about NaNs/unordered "a < b" is *not* the same as "!(a >= b)"). But there is also a number of bugs I want to solve, especially one related to the misplacement of phi-node and another about missing reloads. For the moment I think we can pretend that all the fp values we deal with are ordered ones. -- Luc -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Mar 24, 2017 at 4:54 PM, Luc Van Oostenryck <luc.vanoostenryck@gmail.com> wrote: > Yes, indeed. > I've some plan to add better handling of floating-point and the compare > is part of it. It'll need a new set of instructions to do it correctly > (precisely > because for fp numbers once you care about NaNs/unordered "a < b" is *not* > the same as "!(a >= b)"). > But there is also a number of bugs I want to solve, especially one related to > the misplacement of phi-node and another about missing reloads. For the moment > I think we can pretend that all the fp values we deal with are ordered ones. Can we detect it is the floating point type then avoid doing the compare swap for floating point? Chris -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Mar 25, 2017 at 04:35:39PM -0700, Christopher Li wrote: > On Fri, Mar 24, 2017 at 4:54 PM, Luc Van Oostenryck > <luc.vanoostenryck@gmail.com> wrote: > > Yes, indeed. > > I've some plan to add better handling of floating-point and the compare > > is part of it. It'll need a new set of instructions to do it correctly > > (precisely > > because for fp numbers once you care about NaNs/unordered "a < b" is *not* > > the same as "!(a >= b)"). > > But there is also a number of bugs I want to solve, especially one related to > > the misplacement of phi-node and another about missing reloads. For the moment > > I think we can pretend that all the fp values we deal with are ordered ones. > > Can we detect it is the floating point type then avoid doing the > compare swap for floating point? We were not talking about the swap here but of the 'negate' (and the swaping of the operands is immune to the NaNs/unordered). And in fact, the code which needs the negation of compare's opcode can't be called with floating-points args as this code is part of the simplification made when one of the argument is a constant (and only if the constant is 0 or 1). And by constant we mean here a PSEUDO_VAL, which can never be part of a floating-point operation. So in no cases can we have a problem because of that. -- Luc -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/simplify.c b/simplify.c index e3296a830..deacf97b4 100644 --- a/simplify.c +++ b/simplify.c @@ -403,28 +403,27 @@ static int simplify_mul_div(struct instruction *insn, long long value) return 0; } -static int compare_opcode(int opcode, int inverse) +static int negate_compare_opcode(int opcode, int inverse) { - if (!inverse) - return opcode; - - switch (opcode) { - case OP_SET_EQ: return OP_SET_NE; - case OP_SET_NE: return OP_SET_EQ; - - case OP_SET_LT: return OP_SET_GE; - case OP_SET_LE: return OP_SET_GT; - case OP_SET_GT: return OP_SET_LE; - case OP_SET_GE: return OP_SET_LT; + static const unsigned char opcode_tbl[] = { + [OP_SET_EQ - OP_BINCMP] = OP_SET_NE, + [OP_SET_NE - OP_BINCMP] = OP_SET_EQ, + [OP_SET_GT - OP_BINCMP] = OP_SET_LE, + [OP_SET_GE - OP_BINCMP] = OP_SET_LT, + [OP_SET_LE - OP_BINCMP] = OP_SET_GT, + [OP_SET_LT - OP_BINCMP] = OP_SET_GE, + [OP_SET_A - OP_BINCMP] = OP_SET_BE, + [OP_SET_AE - OP_BINCMP] = OP_SET_B , + [OP_SET_BE - OP_BINCMP] = OP_SET_A , + [OP_SET_B - OP_BINCMP] = OP_SET_AE, + }; - case OP_SET_A: return OP_SET_BE; - case OP_SET_AE: return OP_SET_B; - case OP_SET_B: return OP_SET_AE; - case OP_SET_BE: return OP_SET_A; + assert(opcode >= OP_BINCMP && opcode <= OP_BINCMP_END); - default: + if (!inverse) return opcode; - } + + return opcode_tbl[opcode - OP_BINCMP]; } static int swap_compare_opcode(int opcode) @@ -474,7 +473,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 = negate_compare_opcode(opcode, inverse); use_pseudo(insn, src1, &insn->src1); use_pseudo(insn, src2, &insn->src2); remove_usage(old, &insn->src1);
More precisely, use a table to get the opcdoe corresponding to the negated compare and use a more explicit name for the function. Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> --- simplify.c | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-)