[PATCH] sparse, llvm: Simplify comparison op code generation

[Pekka Enberg <penberg@xxxxxxxxxx>]

Linus writes:

  On Tue, Nov 22, 2011 at 9:40 AM, Pekka Enberg <penberg@xxxxxxxxxx> wrote:
  > This patch implements LLVM code generation for OP_SET_LE, OP_SET_GE, OP_SET_BE,
  > and OP_SET_AE.

  Ugh.

  Can't you just do it with a single statement like

     target = LLVMBuildICmp(fn->builder, translate_op(op), lhs, rhs,
target_name);

  instead of having that case-statement where every case does the same thing?

  The translate_op() thing should be trivial too, just something like

    static int translate_op(int sparse_op)
    {
        static const int trans_tbl[] = {
            .[OP_SET_LE] = LLVMIntSLE,
        ...
         };
         return trans_tbl[sparse_op];
    }

  or whatever. No?

Suggested-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Cc: Christopher Li <sparse@xxxxxxxxxxx>
Cc: Jeff Garzik <jgarzik@xxxxxxxxxx>
Signed-off-by: Pekka Enberg <penberg@xxxxxxxxxx>
---
 sparse-llvm.c |   56 +++++++++++++++++++++++---------------------------------
 1 files changed, 23 insertions(+), 33 deletions(-)

diff --git a/sparse-llvm.c b/sparse-llvm.c
index 700a7a4..4ef02a1 100644
--- a/sparse-llvm.c
+++ b/sparse-llvm.c
@@ -391,6 +391,24 @@ static LLVMTypeRef pseudo_type(struct function *fn, struct instruction *insn, ps
 	return result;
 }
 
+static LLVMIntPredicate translate_op(int opcode)
+{
+	static const LLVMIntPredicate trans_tbl[] = {
+		[OP_SET_EQ]	= LLVMIntEQ,
+		[OP_SET_NE]	= LLVMIntNE,
+		[OP_SET_LE]	= LLVMIntSLE,
+		[OP_SET_GE]	= LLVMIntSGE,
+		[OP_SET_LT]	= LLVMIntSLT,
+		[OP_SET_GT]	= LLVMIntSGT,
+		[OP_SET_B]	= LLVMIntULT,
+		[OP_SET_A]	= LLVMIntUGT,
+		[OP_SET_BE]	= LLVMIntULE,
+		[OP_SET_AE]	= LLVMIntUGE,
+	};
+
+	return trans_tbl[opcode];
+}
+
 static void output_op_binary(struct function *fn, struct instruction *insn)
 {
 	LLVMValueRef lhs, rhs, target;
@@ -493,40 +511,12 @@ static void output_op_binary(struct function *fn, struct instruction *insn)
 	}
 
 	/* Binary comparison */
-	case OP_SET_EQ:
-		assert(!symbol_is_fp_type(insn->type));
-		target = LLVMBuildICmp(fn->builder, LLVMIntEQ, lhs, rhs, target_name);
-		break;
-	case OP_SET_NE:
-		assert(!symbol_is_fp_type(insn->type));
-		target = LLVMBuildICmp(fn->builder, LLVMIntNE, lhs, rhs, target_name);
-		break;
-	case OP_SET_LE:
-		target = LLVMBuildICmp(fn->builder, LLVMIntSLE, lhs, rhs, target_name);
-		break;
-	case OP_SET_GE:
-		target = LLVMBuildICmp(fn->builder, LLVMIntSGE, lhs, rhs, target_name);
-		break;
-	case OP_SET_LT:
-		assert(!symbol_is_fp_type(insn->type));
-		target = LLVMBuildICmp(fn->builder, LLVMIntSLT, lhs, rhs, target_name);
-		break;
-	case OP_SET_GT:
-		assert(!symbol_is_fp_type(insn->type));
-		target = LLVMBuildICmp(fn->builder, LLVMIntSGT, lhs, rhs, target_name);
-		break;
-	case OP_SET_B:
-		target = LLVMBuildICmp(fn->builder, LLVMIntULT, lhs, rhs, target_name);
-		break;
-	case OP_SET_A:
-		target = LLVMBuildICmp(fn->builder, LLVMIntUGT, lhs, rhs, target_name);
-		break;
-	case OP_SET_BE:
-		target = LLVMBuildICmp(fn->builder, LLVMIntULE, lhs, rhs, target_name);
-		break;
-	case OP_SET_AE:
-		target = LLVMBuildICmp(fn->builder, LLVMIntUGE, lhs, rhs, target_name);
+	case OP_BINCMP ... OP_BINCMP_END: {
+		LLVMIntPredicate op = translate_op(insn->opcode);
+
+		target = LLVMBuildICmp(fn->builder, op, lhs, rhs, target_name);
 		break;
+	}
 	default:
 		assert(0);
 		break;
-- 
1.7.6.4

--
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