[PATCH 0/3] fix loading of partially defined bitfield

[PATCH 0/3] fix loading of partially defined bitfield

[Luc Van Oostenryck]

The goal of this series is to fix the problem present in sparse
when a bitfield in an uninitialized automatic variable is first
set then read-back.

In this case the bitfield itself is initialized but not the
remaining of the structure/word and sparse was not smart
enough to handle it.

This is for testing only and the patches are not meant to 
be applied on the main tree becasue it need the new SSA
construction to handle the undefined vars correctly.

(two other simplifications is also needed to make the
generated code a no-op as it should be but not yet
implemented).

The whole code is available in the git repository at:

  git://github.com/lucvoo/sparse.git fix-loading-partialy-defined-bitfields

----------------------------------------------------------------
Luc Van Oostenryck (3):
      new helper: def_opcode()
      change loading of bitfields
      simplify ((A & M') | B ) & M when M' & M == 0

 linearize.c                | 15 +++++++-----
 simplify.c                 | 61 ++++++++++++++++++++++++++++++++++++++++------
 validation/bitfield-size.c |  6 +++--
 3 files changed, 66 insertions(+), 16 deletions(-)

[PATCH 1/3] new helper: def_opcode()

[Luc Van Oostenryck]

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 simplify.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/simplify.c b/simplify.c
index f292fd5b6..cfb6b2cda 100644
--- a/simplify.c
+++ b/simplify.c
@@ -380,6 +380,13 @@ static int replace_with_pseudo(struct instruction *insn, pseudo_t pseudo)
 	return REPEAT_CSE;
 }
 
+static inline int def_opcode(pseudo_t p)
+{
+	if (p->type != PSEUDO_REG)
+		return -1;
+	return p->def->opcode;
+}
+
 static unsigned int value_size(long long value)
 {
 	value >>= 8;
@@ -404,9 +411,9 @@ static unsigned int operand_size(struct instruction *insn, pseudo_t pseudo)
 {
 	unsigned int size = insn->size;
 
-	if (pseudo->type == PSEUDO_REG) {
+	if (def_opcode(pseudo) == OP_CAST) {
 		struct instruction *src = pseudo->def;
-		if (src && src->opcode == OP_CAST && src->orig_type) {
+		if (src->orig_type) {
 			unsigned int orig_size = src->orig_type->bit_size;
 			if (orig_size < size)
 				size = orig_size;
@@ -822,13 +829,11 @@ static int simplify_associative_binop(struct instruction *insn)
 
 	if (!simple_pseudo(insn->src2))
 		return 0;
-	if (pseudo->type != PSEUDO_REG)
+	if (def_opcode(pseudo) != insn->opcode)
 		return 0;
 	def = pseudo->def;
 	if (def == insn)
 		return 0;
-	if (def->opcode != insn->opcode)
-		return 0;
 	if (!simple_pseudo(def->src2))
 		return 0;
 	if (ptr_list_size((struct ptr_list *)def->target->users) != 1)
@@ -993,9 +998,9 @@ static int simplify_cast(struct instruction *insn)
 	}
 
 	/* A cast of a "and" might be a no-op.. */
-	if (src->type == PSEUDO_REG) {
+	if (def_opcode(src) == OP_AND) {
 		struct instruction *def = src->def;
-		if (def->opcode == OP_AND && def->size >= size) {
+		if (def->size >= size) {
 			pseudo_t val = def->src2;
 			if (val->type == PSEUDO_VAL) {
 				unsigned long long value = val->value;
-- 
2.13.2

[PATCH 2/3] change loading of bitfields

[Luc Van Oostenryck]

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 linearize.c                | 15 +++++++++------
 validation/bitfield-size.c |  6 ++++--
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/linearize.c b/linearize.c
index af46191f1..b06cf4e19 100644
--- a/linearize.c
+++ b/linearize.c
@@ -1077,14 +1077,17 @@ static pseudo_t linearize_load_gen(struct entrypoint *ep, struct access_data *ad
 {
 	struct symbol *ctype = ad->result_type;
 	pseudo_t new = add_load(ep, ad);
+	unsigned int off = ctype->bit_offset;
+	unsigned int siz = ctype->bit_size;
 
-	if (ctype->bit_offset) {
-		pseudo_t shift = value_pseudo(ctype->bit_offset);
-		pseudo_t newval = add_binary_op(ep, ad->source_type, OP_LSR, new, shift);
-		new = newval;
+	if (siz != type_size(ad->source_type)) {
+		pseudo_t mask = value_pseudo(((1ULL << siz) - 1) << off);
+		new = add_binary_op(ep, ad->source_type, OP_AND, new, mask);
+	}
+	if (off) {
+		pseudo_t shift = value_pseudo(off);
+		new = add_binary_op(ep, ad->source_type, OP_LSR, new, shift);
 	}
-	if (ctype->bit_size != type_size(ad->source_type))
-		new = cast_pseudo(ep, new, ad->source_type, ad->result_type);
 	return new;
 }
 
diff --git a/validation/bitfield-size.c b/validation/bitfield-size.c
index ce78ecf21..c8c94bb15 100644
--- a/validation/bitfield-size.c
+++ b/validation/bitfield-size.c
@@ -35,7 +35,9 @@ unsigned int get_pbfi_b(struct bfi *bf) { return bf->b; }
  * check-command: test-linearize -Wno-decl $file
  * check-output-ignore
  *
- * check-output-pattern-24-times: cast\\.
- * check-output-pattern-12-times: cast\\.4
+ * check-output-excludes: cast\\.4
+ * check-output-pattern-6-times: cast\\.
  * check-output-pattern-6-times: lsr\\..*\\$6
+ * check-output-pattern-6-times: and\\..*\\$15
+ * check-output-pattern-6-times: and\\..*\\$960
  */
-- 
2.13.2

[PATCH 3/3] simplify ((A & M') | B ) & M when M' & M == 0

[Luc Van Oostenryck]

Reported-by: Dibyendu Majumdar <mobile@majumdar.org.uk>

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 simplify.c | 42 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/simplify.c b/simplify.c
index cfb6b2cda..3fb2c8d8f 100644
--- a/simplify.c
+++ b/simplify.c
@@ -532,6 +532,46 @@ static int simplify_seteq_setne(struct instruction *insn, long long value)
 	}
 }
 
+static int simplify_and_or_mask(struct instruction *insn, pseudo_t and, pseudo_t other, unsigned long long mask)
+{
+	struct instruction *def = and->def;
+	pseudo_t old;
+
+	if (!constant(def->src2))
+		return 0;
+	if (def->src2->value & mask)
+		return 0;
+	old = insn->src1;
+	use_pseudo(insn, other, &insn->src1);
+	remove_usage(old, &insn->src1);
+	return REPEAT_CSE;
+}
+
+static int simplify_constant_mask(struct instruction *insn, unsigned long long mask)
+{
+	struct instruction *left;
+	pseudo_t src1, src2;
+
+	switch (def_opcode(insn->src1)) {
+	case OP_OR:
+		// Let's handle ((A & M') | B ) & M
+		// or           (B | (A & M')) & M
+		// when M' & M == 0
+		left = insn->src1->def;
+		src1 = left->src1;
+		src2 = left->src2;
+		if (def_opcode(src1) == OP_AND)
+			return simplify_and_or_mask(insn, src1, src2, mask);
+		if (def_opcode(src2) == OP_AND)
+			return simplify_and_or_mask(insn, src2, src1, mask);
+		break;
+
+	default:
+		break;
+	}
+	return 0;
+}
+
 static int simplify_constant_rightside(struct instruction *insn)
 {
 	long long value = insn->src2->value;
@@ -576,7 +616,7 @@ static int simplify_constant_rightside(struct instruction *insn)
 	case OP_AND:
 		if (!value)
 			return replace_with_pseudo(insn, insn->src2);
-		return 0;
+		return simplify_constant_mask(insn, value);
 
 	case OP_SET_NE:
 	case OP_SET_EQ:
-- 
2.13.2

Re: [PATCH 0/3] fix loading of partially defined bitfield

[Christopher Li]

On Sun, Aug 6, 2017 at 9:33 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> The goal of this series is to fix the problem present in sparse
> when a bitfield in an uninitialized automatic variable is first
> set then read-back.
>
> In this case the bitfield itself is initialized but not the
> remaining of the structure/word and sparse was not smart
> enough to handle it.
>
> This is for testing only and the patches are not meant to
> be applied on the main tree becasue it need the new SSA
> construction to handle the undefined vars correctly.
>
> (two other simplifications is also needed to make the
> generated code a no-op as it should be but not yet
> implemented).
>
> The whole code is available in the git repository at:
>
>   git://github.com/lucvoo/sparse.git fix-loading-partialy-defined-bitfields

Is this for RC5?

Chris

Re: [PATCH 0/3] fix loading of partially defined bitfield

[Christopher Li]

On Sun, Aug 6, 2017 at 9:59 PM, Christopher Li <sparse@chrisli.org> wrote:
> On Sun, Aug 6, 2017 at 9:33 PM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>> The goal of this series is to fix the problem present in sparse
>> when a bitfield in an uninitialized automatic variable is first
>> set then read-back.
>>
>> In this case the bitfield itself is initialized but not the
>> remaining of the structure/word and sparse was not smart
>> enough to handle it.
>>
>> This is for testing only and the patches are not meant to
>> be applied on the main tree becasue it need the new SSA
>> construction to handle the undefined vars correctly.
>>
>> (two other simplifications is also needed to make the
>> generated code a no-op as it should be but not yet
>> implemented).
>>
>> The whole code is available in the git repository at:
>>
>>   git://github.com/lucvoo/sparse.git fix-loading-partialy-defined-bitfields
>
> Is this for RC5?

Sorry I did not read your description very well.

You said it is not for main line yet.

Chris