Re: [PATCH v2] implement constant-folding in __builtin_bswap*()

[Christopher Li <sparse@chrisli.org>]

[-- Attachment #1: Type: text/plain, Size: 948 bytes --]

On Tue, Nov 22, 2016 at 9:15 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>>
>> I will see if I can hack up some thing very quick.

I get it working in the end. Not too far from what I thought.
Patch attach here for review.

>
> I think it would be best to do any change related tos handling
> of constant expressions on to of Nicolai Stange's serie:
>         http://marc.info/?l=linux-sparse&m=145429372932235

Yes, it is on my patch list. That series can still apply clean.
I just check it.

My current strategy is apply other patches which have impact
on the sparse warning first. Otherwise my anxiety level is too
high, make me hard to pick up and hack sparse. That integer
series needs a relative big chunk of time to review it. For me
it is more important to keep on making progress rather than
solve all problem in one blow.

I think I have been catch up with most of the patches. Should be
able to work on that soon.

Chris

[-- Attachment #2: implement-builtin-bswap.patch --]
[-- Type: text/x-patch, Size: 6130 bytes --]

diff --git a/expand.c b/expand.c
index 0f6720c..1ef95ea 100644
--- a/expand.c
+++ b/expand.c
@@ -802,6 +802,44 @@ int expand_safe_p(struct expression *expr, int cost)
 	return 0;
 }
 
+/* The arguments are constant if the cost of all of them is zero */
+int expand_bswap(struct expression *expr, int cost)
+{
+	struct symbol *sym;
+	struct expression_list *args = expr->args;
+	long long input;
+
+	if (cost)
+		return cost;
+
+	sym = expr->fn->ctype;
+	if (expression_list_size(args) != 1) {
+		sparse_error(expr->pos, "not enough arguments for function %s",
+				show_ident(sym->ident));
+		return SIDE_EFFECTS;
+	}
+
+	input = const_expression_value(first_expression(args));
+
+	if (sym->ident == &__builtin_bswap16_ident) {
+		expr->value = __builtin_bswap16(input);
+		expr->ctype = &ushort_ctype;
+	} else if (sym->ident == &__builtin_bswap32_ident) {
+		expr->value = __builtin_bswap32(input);
+		expr->ctype = &uint_ctype;
+	} else if (sym->ident == &__builtin_bswap64_ident) {
+		expr->value = __builtin_bswap64(input);
+		expr->ctype = &ullong_ctype;
+	} else {
+		die("Unexpected __builtin_bswap symbol %s\n", show_ident(sym->ident));
+	}
+
+	expr->type = EXPR_VALUE;
+	expr->taint = 0;
+	return 0;
+}
+
+
 /*
  * expand a call expression with a symbol. This
  * should expand builtins.
diff --git a/ident-list.h b/ident-list.h
index b65b667..a683f6c 100644
--- a/ident-list.h
+++ b/ident-list.h
@@ -35,6 +35,9 @@ IDENT_RESERVED(__sizeof_ptr__);
 IDENT_RESERVED(__builtin_types_compatible_p);
 IDENT_RESERVED(__builtin_offsetof);
 IDENT_RESERVED(__label__);
+IDENT(__builtin_bswap16);
+IDENT(__builtin_bswap32);
+IDENT(__builtin_bswap64);
 
 /* Attribute names */
 IDENT(packed); IDENT(__packed__);
diff --git a/lib.c b/lib.c
index 2660575..7e35049 100644
--- a/lib.c
+++ b/lib.c
@@ -819,40 +819,6 @@ void declare_builtin_functions(void)
 	add_pre_buffer("extern int __builtin_popcountl(unsigned long);\n");
 	add_pre_buffer("extern int __builtin_popcountll(unsigned long long);\n");
 
-	/* And byte swaps.. */
-	add_pre_buffer("extern unsigned short ____builtin_bswap16(unsigned short);\n");
-	add_pre_buffer("extern unsigned int ____builtin_bswap32(unsigned int);\n");
-	add_pre_buffer("extern unsigned long long ____builtin_bswap64(unsigned long long);\n");
-	add_pre_buffer("#define __sparse_constant_swab16(x) ((unsigned short)("
-		       "	(((unsigned short)(x) & (unsigned short)0x00ffU) << 8) |"
-		       "	(((unsigned short)(x) & (unsigned short)0xff00U) >> 8)))\n");
-	add_pre_buffer("#define __sparse_constant_swab32(x) ((unsigned int)("
-		       "	(((unsigned int)(x) & (unsigned int)0x000000ffUL) << 24) |"
-		       "	(((unsigned int)(x) & (unsigned int)0x0000ff00UL) <<  8) |"
-		       "	(((unsigned int)(x) & (unsigned int)0x00ff0000UL) >>  8) |"
-		       "	(((unsigned int)(x) & (unsigned int)0xff000000UL) >> 24)))\n");
-	add_pre_buffer("#define __sparse_constant_swab64(x) ((unsigned long long)("
-		       "	(((unsigned long long)(x) & (unsigned long long)0x00000000000000ffULL) << 56) |"
-		       "	(((unsigned long long)(x) & (unsigned long long)0x000000000000ff00ULL) << 40) |"
-		       "	(((unsigned long long)(x) & (unsigned long long)0x0000000000ff0000ULL) << 24) |"
-		       "	(((unsigned long long)(x) & (unsigned long long)0x00000000ff000000ULL) <<  8) |"
-		       "	(((unsigned long long)(x) & (unsigned long long)0x000000ff00000000ULL) >>  8) |"
-		       "	(((unsigned long long)(x) & (unsigned long long)0x0000ff0000000000ULL) >> 24) |"
-		       "	(((unsigned long long)(x) & (unsigned long long)0x00ff000000000000ULL) >> 40) |"
-		       "	(((unsigned long long)(x) & (unsigned long long)0xff00000000000000ULL) >> 56)))\n");
-	add_pre_buffer("#define __builtin_bswap16(x)"
-		       "	(__builtin_constant_p((unsigned short)(x)) ?"
-		       "	__sparse_constant_swab16(x) :"
-		       "	____builtin_bswap16(x))\n");
-	add_pre_buffer("#define __builtin_bswap32(x)"
-		       "	(__builtin_constant_p((unsigned int)(x)) ?"
-		       "	__sparse_constant_swab32(x) :"
-		       "	____builtin_bswap32(x))\n");
-	add_pre_buffer("#define __builtin_bswap64(x)"
-		       "	(__builtin_constant_p((unsigned long long)(x)) ?"
-		       "	__sparse_constant_swab64(x) :"
-		       "	____builtin_bswap64(x))\n");
-
 	/* And atomic memory access functions.. */
 	add_pre_buffer("extern int __sync_fetch_and_add(void *, ...);\n");
 	add_pre_buffer("extern int __sync_fetch_and_sub(void *, ...);\n");
diff --git a/lib.h b/lib.h
index b778bdc..306ee45 100644
--- a/lib.h
+++ b/lib.h
@@ -200,6 +200,11 @@ static inline struct instruction *first_instruction(struct instruction_list *hea
 	return first_ptr_list((struct ptr_list *)head);
 }
 
+static inline struct expression *first_expression(struct expression_list *head)
+{
+	return first_ptr_list((struct ptr_list *)head);
+}
+
 static inline pseudo_t first_pseudo(struct pseudo_list *head)
 {
 	return first_ptr_list((struct ptr_list *)head);
diff --git a/symbol.c b/symbol.c
index 92a7a62..e57f207 100644
--- a/symbol.c
+++ b/symbol.c
@@ -773,6 +773,11 @@ static struct symbol_op choose_op = {
 	.args = arguments_choose,
 };
 
+static struct symbol_op bswap_op = {
+	.evaluate = evaluate_to_integer,
+	.expand = expand_bswap
+};
+
 /*
  * Builtin functions
  */
@@ -788,6 +793,9 @@ static struct sym_init {
 	{ "__builtin_warning", &builtin_fn_type, MOD_TOPLEVEL, &warning_op },
 	{ "__builtin_expect", &builtin_fn_type, MOD_TOPLEVEL, &expect_op },
 	{ "__builtin_choose_expr", &builtin_fn_type, MOD_TOPLEVEL, &choose_op },
+	{ "__builtin_bswap16", &builtin_fn_type, MOD_TOPLEVEL, &bswap_op },
+	{ "__builtin_bswap32", &builtin_fn_type, MOD_TOPLEVEL, &bswap_op },
+	{ "__builtin_bswap64", &builtin_fn_type, MOD_TOPLEVEL, &bswap_op },
 	{ NULL,		NULL,		0 }
 };
 
diff --git a/symbol.h b/symbol.h
index 9b3f160..48bbfce 100644
--- a/symbol.h
+++ b/symbol.h
@@ -130,6 +130,7 @@ struct symbol_op {
 
 extern int expand_safe_p(struct expression *expr, int cost);
 extern int expand_constant_p(struct expression *expr, int cost);
+extern int expand_bswap(struct expression *expr, int cost);
 
 #define SYM_ATTR_WEAK		0
 #define SYM_ATTR_NORMAL		1