[PATCH 0/4] small reorganization of OP_SLICE

[PATCH 0/4] small reorganization of OP_SLICE

[Luc Van Oostenryck]

Slightly reorganize OP_SLICE instructions in preparation for
some incoming changes involving them.

Luc Van Oostenryck (4):
  slice: remove unneeded len from OP_SLICE
  slice: remove unneeded nr_nrbits from EXPR_SLICE
  slice: OP_SLICE needs the source's type: make it a kind of unop
  slice: display the source's size, like for unops

 Documentation/IR.rst | 2 +-
 evaluate.c           | 1 -
 expression.h         | 2 +-
 linearize.c          | 6 +++---
 linearize.h          | 5 +----
 liveness.c           | 5 +----
 show-parse.c         | 2 +-
 7 files changed, 8 insertions(+), 15 deletions(-)

-- 
2.30.0

[PATCH 1/4] slice: remove unneeded len from OP_SLICE

[Luc Van Oostenryck]

OP_SLICE::len is necessarily equal to the result size.
So remove this redundancy.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 Documentation/IR.rst | 2 +-
 linearize.c          | 3 +--
 linearize.h          | 2 +-
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/Documentation/IR.rst b/Documentation/IR.rst
index 38df84ff3954..c7db32073361 100644
--- a/Documentation/IR.rst
+++ b/Documentation/IR.rst
@@ -408,7 +408,7 @@ Others
 	Extract a "slice" from an aggregate.
 
 	* .base: (pseudo_t) aggregate (alias .src)
-	* .from, .len: offet & size of the "slice" within the aggregate
+	* .from: offet of the "slice" within the aggregate
 	* .target: result
 	* .type: type of .target
 
diff --git a/linearize.c b/linearize.c
index 0c9b0e59cc4b..96a717bc2909 100644
--- a/linearize.c
+++ b/linearize.c
@@ -470,7 +470,7 @@ const char *show_instruction(struct instruction *insn)
 		break;
 
 	case OP_SLICE:
-		buf += sprintf(buf, "%s <- %s, %d, %d", show_pseudo(insn->target), show_pseudo(insn->base), insn->from, insn->len);
+		buf += sprintf(buf, "%s <- %s, %d", show_pseudo(insn->target), show_pseudo(insn->base), insn->from);
 		break;
 
 	case OP_NOT: case OP_NEG:
@@ -1239,7 +1239,6 @@ static pseudo_t linearize_slice(struct entrypoint *ep, struct expression *expr)
 
 	insn->target = new;
 	insn->from = expr->r_bitpos;
-	insn->len = expr->r_nrbits;
 	use_pseudo(insn, pre, &insn->base);
 	add_one_insn(ep, insn);
 	return new;
diff --git a/linearize.h b/linearize.h
index cf0cf066a8e5..4d83675caaf1 100644
--- a/linearize.h
+++ b/linearize.h
@@ -129,7 +129,7 @@ struct instruction {
 		};
 		struct /* slice */ {
 			pseudo_t base;
-			unsigned from, len;
+			unsigned from;
 		};
 		struct /* setval */ {
 			struct expression *val;
-- 
2.30.0

Re: [PATCH 1/4] slice: remove unneeded len from OP_SLICE

[Ramsay Jones]

On 25/02/2021 23:39, Luc Van Oostenryck wrote:
> OP_SLICE::len is necessarily equal to the result size.
> So remove this redundancy.
> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
>  Documentation/IR.rst | 2 +-
>  linearize.c          | 3 +--
>  linearize.h          | 2 +-
>  3 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/IR.rst b/Documentation/IR.rst
> index 38df84ff3954..c7db32073361 100644
> --- a/Documentation/IR.rst
> +++ b/Documentation/IR.rst
> @@ -408,7 +408,7 @@ Others
>  	Extract a "slice" from an aggregate.
>  
>  	* .base: (pseudo_t) aggregate (alias .src)
> -	* .from, .len: offet & size of the "slice" within the aggregate
> +	* .from: offet of the "slice" within the aggregate

s/offet/offset/

ATB,
Ramsay Jones

>  	* .target: result
>  	* .type: type of .target
>  
> diff --git a/linearize.c b/linearize.c
> index 0c9b0e59cc4b..96a717bc2909 100644
> --- a/linearize.c
> +++ b/linearize.c
> @@ -470,7 +470,7 @@ const char *show_instruction(struct instruction *insn)
>  		break;
>  
>  	case OP_SLICE:
> -		buf += sprintf(buf, "%s <- %s, %d, %d", show_pseudo(insn->target), show_pseudo(insn->base), insn->from, insn->len);
> +		buf += sprintf(buf, "%s <- %s, %d", show_pseudo(insn->target), show_pseudo(insn->base), insn->from);
>  		break;
>  
>  	case OP_NOT: case OP_NEG:
> @@ -1239,7 +1239,6 @@ static pseudo_t linearize_slice(struct entrypoint *ep, struct expression *expr)
>  
>  	insn->target = new;
>  	insn->from = expr->r_bitpos;
> -	insn->len = expr->r_nrbits;
>  	use_pseudo(insn, pre, &insn->base);
>  	add_one_insn(ep, insn);
>  	return new;
> diff --git a/linearize.h b/linearize.h
> index cf0cf066a8e5..4d83675caaf1 100644
> --- a/linearize.h
> +++ b/linearize.h
> @@ -129,7 +129,7 @@ struct instruction {
>  		};
>  		struct /* slice */ {
>  			pseudo_t base;
> -			unsigned from, len;
> +			unsigned from;
>  		};
>  		struct /* setval */ {
>  			struct expression *val;
> 

[PATCH 2/4] slice: remove unneeded nr_nrbits from EXPR_SLICE

[Luc Van Oostenryck]

EXPR_SLICE::r_nrbits is necessarily equal to its type's bit size.
So remove this redundancy.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 evaluate.c   | 1 -
 expression.h | 2 +-
 show-parse.c | 2 +-
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/evaluate.c b/evaluate.c
index 41871e18503a..e13edf5488b4 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -2170,7 +2170,6 @@ static struct symbol *evaluate_member_dereference(struct expression *expr)
 		}
 		expr->r_bitpos += bytes_to_bits(offset);
 		expr->type = EXPR_SLICE;
-		expr->r_nrbits = member->bit_size;
 		expr->r_bitpos += member->bit_offset;
 		expr->ctype = member;
 		return member;
diff --git a/expression.h b/expression.h
index 3e9e9d852c27..f733c07697c8 100644
--- a/expression.h
+++ b/expression.h
@@ -206,7 +206,7 @@ struct expression {
 		// EXPR_SLICE
 		struct /* slice */ {
 			struct expression *base;
-			unsigned r_bitpos, r_nrbits;
+			unsigned r_bitpos;
 		};
 		// EXPR_CAST, EXPR_FORCE_CAST, EXPR_IMPLIED_CAST,
 		// EXPR_SIZEOF, EXPR_ALIGNOF and EXPR_PTRSIZEOF
diff --git a/show-parse.c b/show-parse.c
index 3ab8ec8f1894..e2fc18bb4b3d 100644
--- a/show-parse.c
+++ b/show-parse.c
@@ -819,7 +819,7 @@ static int show_slice(struct expression *expr)
 {
 	int target = show_expression(expr->base);
 	int new = new_pseudo();
-	printf("\tslice.%d\t\tv%d,v%d,%d\n", expr->r_nrbits, target, new, expr->r_bitpos);
+	printf("\tslice.%d\t\tv%d,v%d,%d\n", expr->ctype->bit_size, target, new, expr->r_bitpos);
 	return new;
 }
 
-- 
2.30.0

[PATCH 3/4] slice: OP_SLICE needs the source's type: make it a kind of unop

[Luc Van Oostenryck]

OP_SLICE's source's type is needed for some simplifications.
For example, in some cases it can be simplified into OP_TRUNC.

So, merge its representation with the one for unops which also
need the source's type.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 linearize.c | 5 +++--
 linearize.h | 5 +----
 liveness.c  | 5 +----
 3 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/linearize.c b/linearize.c
index 96a717bc2909..7ab69d3ac968 100644
--- a/linearize.c
+++ b/linearize.c
@@ -470,7 +470,7 @@ const char *show_instruction(struct instruction *insn)
 		break;
 
 	case OP_SLICE:
-		buf += sprintf(buf, "%s <- %s, %d", show_pseudo(insn->target), show_pseudo(insn->base), insn->from);
+		buf += sprintf(buf, "%s <- %s, %d", show_pseudo(insn->target), show_pseudo(insn->src), insn->from);
 		break;
 
 	case OP_NOT: case OP_NEG:
@@ -1239,7 +1239,8 @@ static pseudo_t linearize_slice(struct entrypoint *ep, struct expression *expr)
 
 	insn->target = new;
 	insn->from = expr->r_bitpos;
-	use_pseudo(insn, pre, &insn->base);
+	insn->orig_type = expr->base->ctype;
+	use_pseudo(insn, pre, &insn->src);
 	add_one_insn(ep, insn);
 	return new;
 }
diff --git a/linearize.h b/linearize.h
index 4d83675caaf1..429f4797e359 100644
--- a/linearize.h
+++ b/linearize.h
@@ -113,6 +113,7 @@ struct instruction {
 		};
 		struct /* unops */ {
 			pseudo_t src;
+			unsigned from;			/* slice */
 			struct symbol *orig_type;	/* casts */
 		};
 		struct /* memops */ {
@@ -127,10 +128,6 @@ struct instruction {
 			pseudo_t _src1, _src2;		// alias .src[12]
 			struct symbol *itype;		// input operands' type
 		};
-		struct /* slice */ {
-			pseudo_t base;
-			unsigned from;
-		};
 		struct /* setval */ {
 			struct expression *val;
 		};
diff --git a/liveness.c b/liveness.c
index 30a9a5b6b169..755509e59b52 100644
--- a/liveness.c
+++ b/liveness.c
@@ -76,6 +76,7 @@ static void track_instruction_usage(struct basic_block *bb, struct instruction *
 	/* Uni */
 	case OP_UNOP ... OP_UNOP_END:
 	case OP_SYMADDR:
+	case OP_SLICE:
 		USES(src1); DEFINES(target);
 		break;
 
@@ -121,10 +122,6 @@ static void track_instruction_usage(struct basic_block *bb, struct instruction *
 		} END_FOR_EACH_PTR(pseudo);
 		break;
 
-	case OP_SLICE:
-		USES(base); DEFINES(target);
-		break;
-
 	case OP_ASM:
 		asm_liveness(bb, insn, def, use);
 		break;
-- 
2.30.0

Re: [PATCH 3/4] slice: OP_SLICE needs the source's type: make it a kind of unop

[Ramsay Jones]

On 25/02/2021 23:39, Luc Van Oostenryck wrote:
> OP_SLICE's source's type is needed for some simplifications.
> For example, in some cases it can be simplified into OP_TRUNC.
> 
> So, merge its representation with the one for unops which also
> need the source's type.
> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
>  linearize.c | 5 +++--
>  linearize.h | 5 +----
>  liveness.c  | 5 +----
>  3 files changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/linearize.c b/linearize.c
> index 96a717bc2909..7ab69d3ac968 100644
> --- a/linearize.c
> +++ b/linearize.c
> @@ -470,7 +470,7 @@ const char *show_instruction(struct instruction *insn)
>  		break;
>  
>  	case OP_SLICE:
> -		buf += sprintf(buf, "%s <- %s, %d", show_pseudo(insn->target), show_pseudo(insn->base), insn->from);
> +		buf += sprintf(buf, "%s <- %s, %d", show_pseudo(insn->target), show_pseudo(insn->src), insn->from);
>  		break;
>  
>  	case OP_NOT: case OP_NEG:
> @@ -1239,7 +1239,8 @@ static pseudo_t linearize_slice(struct entrypoint *ep, struct expression *expr)
>  
>  	insn->target = new;
>  	insn->from = expr->r_bitpos;
> -	use_pseudo(insn, pre, &insn->base);
> +	insn->orig_type = expr->base->ctype;
> +	use_pseudo(insn, pre, &insn->src);
>  	add_one_insn(ep, insn);
>  	return new;
>  }
> diff --git a/linearize.h b/linearize.h
> index 4d83675caaf1..429f4797e359 100644
> --- a/linearize.h
> +++ b/linearize.h
> @@ -113,6 +113,7 @@ struct instruction {
>  		};
>  		struct /* unops */ {
>  			pseudo_t src;
> +			unsigned from;			/* slice */
>  			struct symbol *orig_type;	/* casts */
>  		};
>  		struct /* memops */ {
> @@ -127,10 +128,6 @@ struct instruction {
>  			pseudo_t _src1, _src2;		// alias .src[12]
>  			struct symbol *itype;		// input operands' type
>  		};
> -		struct /* slice */ {
> -			pseudo_t base;
> -			unsigned from;
> -		};
>  		struct /* setval */ {
>  			struct expression *val;
>  		};
> diff --git a/liveness.c b/liveness.c
> index 30a9a5b6b169..755509e59b52 100644
> --- a/liveness.c
> +++ b/liveness.c
> @@ -76,6 +76,7 @@ static void track_instruction_usage(struct basic_block *bb, struct instruction *
>  	/* Uni */
>  	case OP_UNOP ... OP_UNOP_END:
>  	case OP_SYMADDR:
> +	case OP_SLICE:
>  		USES(src1); DEFINES(target);

wouldn't USES(src) be more appropriate? They are not binops.

ATB,
Ramsay Jones

>  		break;
>  
> @@ -121,10 +122,6 @@ static void track_instruction_usage(struct basic_block *bb, struct instruction *
>  		} END_FOR_EACH_PTR(pseudo);
>  		break;
>  
> -	case OP_SLICE:
> -		USES(base); DEFINES(target);
> -		break;
> -
>  	case OP_ASM:
>  		asm_liveness(bb, insn, def, use);
>  		break;
> 

Re: [PATCH 3/4] slice: OP_SLICE needs the source's type: make it a kind of unop

[Luc Van Oostenryck]

On Fri, Feb 26, 2021 at 11:56:58PM +0000, Ramsay Jones wrote:
> On 25/02/2021 23:39, Luc Van Oostenryck wrote:
> > diff --git a/liveness.c b/liveness.c
> > index 30a9a5b6b169..755509e59b52 100644
> > --- a/liveness.c
> > +++ b/liveness.c
> > @@ -76,6 +76,7 @@ static void track_instruction_usage(struct basic_block *bb, struct instruction *
> >  	/* Uni */
> >  	case OP_UNOP ... OP_UNOP_END:
> >  	case OP_SYMADDR:
> > +	case OP_SLICE:
> >  		USES(src1); DEFINES(target);
> 
> wouldn't USES(src) be more appropriate? They are not binops.

Yes, even though they are synonymous (and documented as such) it makes
thinks slightly clearer. I'll change this in a separate patch.

Thank you (also for the 2 other patches),
-- Luc

[PATCH 4/4] slice: display the source's size, like for unops

[Luc Van Oostenryck]

When displaying an OP_SLICE, the width is shown but the size
of the source pseudo is useful to. So display the it, like
done for unops.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 linearize.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linearize.c b/linearize.c
index 7ab69d3ac968..b06c062599ee 100644
--- a/linearize.c
+++ b/linearize.c
@@ -470,7 +470,7 @@ const char *show_instruction(struct instruction *insn)
 		break;
 
 	case OP_SLICE:
-		buf += sprintf(buf, "%s <- %s, %d", show_pseudo(insn->target), show_pseudo(insn->src), insn->from);
+		buf += sprintf(buf, "%s <- (%d) %s, %d", show_pseudo(insn->target), type_size(insn->orig_type), show_pseudo(insn->src), insn->from);
 		break;
 
 	case OP_NOT: case OP_NEG:
-- 
2.30.0

Re: [PATCH 4/4] slice: display the source's size, like for unops

[Ramsay Jones]

On 25/02/2021 23:39, Luc Van Oostenryck wrote:
> When displaying an OP_SLICE, the width is shown but the size
> of the source pseudo is useful to. So display the it, like
> done for unops.

Hmm, s/useful to./useful too./, s/the it,/it/,
s/like.*unops./in a similar manner to the unops./

... if that makes sense! ;-)

ie:
	When displaying an OP_SLICE, the width is shown but the size
	of the source pseudo is useful too. So, display it in a similar
	manner to the unops.


ATB,
Ramsay Jones

> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
>  linearize.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/linearize.c b/linearize.c
> index 7ab69d3ac968..b06c062599ee 100644
> --- a/linearize.c
> +++ b/linearize.c
> @@ -470,7 +470,7 @@ const char *show_instruction(struct instruction *insn)
>  		break;
>  
>  	case OP_SLICE:
> -		buf += sprintf(buf, "%s <- %s, %d", show_pseudo(insn->target), show_pseudo(insn->src), insn->from);
> +		buf += sprintf(buf, "%s <- (%d) %s, %d", show_pseudo(insn->target), type_size(insn->orig_type), show_pseudo(insn->src), insn->from);
>  		break;
>  
>  	case OP_NOT: case OP_NEG:
>