[PATCH 0/6] small changes to ptrlist API

[PATCH 0/6] small changes to ptrlist API

[Luc Van Oostenryck]

This series contains a few changes to the ptrlst API:
* let TYPEOF() return the entry's type instead of the type of its address
* rename TYPEOF() to PTRLIST_TYPE()
* add pop_ptr_list(), a generic macro
* make linearize_ptr_list() generic and rename it to ptr_list_to_array() 
* change the return value of linearize_ptr_list()/ptr_list_to_array() to
  simplify its use.

Luc Van Oostenryck (6):
  ptrlist: ~fix TYPEOF()
  ptrlist: change TYPEOF() into PTRLIST_TYPE()
  ptrlist: add pop_ptr_list()
  ptrlist: use ptr_list_nth() instead of linearize_ptr_list()
  ptrlist: make linearize_ptr_list() generic
  ptrlist: change return value of linearize_ptr_list()/ptr_list_to_array()

 ptrlist.c  | 10 +++++-----
 ptrlist.h  | 22 +++++++++++++++++-----
 simplify.c |  6 +++---
 sparse.c   | 13 +------------
 4 files changed, 26 insertions(+), 25 deletions(-)

-- 
2.30.0

[PATCH 1/6] ptrlist: ~fix TYPEOF()

[Luc Van Oostenryck]

The macro TYPEOF() return the type of the addresses of the pointers
stored in the list. That's one level too much in general.

Change it to simply return the type of the stored pointers.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 ptrlist.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ptrlist.h b/ptrlist.h
index c5fa4cdd94cb..41d9011c8716 100644
--- a/ptrlist.h
+++ b/ptrlist.h
@@ -12,7 +12,7 @@
 
 /* Silly type-safety check ;) */
 #define CHECK_TYPE(head,ptr)		(void)(&(ptr) == &(head)->list[0])
-#define TYPEOF(head)			__typeof__(&(head)->list[0])
+#define TYPEOF(head)			__typeof__((head)->list[0])
 #define VRFY_PTR_LIST(head)		(void)(sizeof((head)->list[0]))
 
 #define LIST_NODE_NR (13)
@@ -251,7 +251,7 @@ extern void __free_ptr_list(struct ptr_list **);
 extern void split_ptr_list_head(struct ptr_list *);
 
 #define DO_INSERT_CURRENT(new, __head, __list, __nr) do {		\
-	TYPEOF(__head) __this, __last;					\
+	TYPEOF(__head) *__this, *__last;				\
 	if (__list->nr == LIST_NODE_NR) {				\
 		split_ptr_list_head((struct ptr_list*)__list);		\
 		if (__nr >= __list->nr) {				\
@@ -270,8 +270,8 @@ extern void split_ptr_list_head(struct ptr_list *);
 } while (0)
 
 #define DO_DELETE_CURRENT(__head, __list, __nr) do {			\
-	TYPEOF(__head) __this = __list->list + __nr;			\
-	TYPEOF(__head) __last = __list->list + __list->nr - 1;		\
+	TYPEOF(__head) *__this = __list->list + __nr;			\
+	TYPEOF(__head) *__last = __list->list + __list->nr - 1;		\
 	while (__this < __last) {					\
 		__this[0] = __this[1];					\
 		__this++;						\
-- 
2.30.0

Re: [PATCH 1/6] ptrlist: ~fix TYPEOF()

[Ramsay Jones]

On 06/03/2021 10:05, Luc Van Oostenryck wrote:
> The macro TYPEOF() return the type of the addresses of the pointers
> stored in the list. That's one level too much in general.
> 
> Change it to simply return the type of the stored pointers.

s/~fix/fix/ in the subject. I read ~ as NOT, so NOT fix, or don't fix. ;-)

ATB,
Ramsay Jones

> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
>  ptrlist.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/ptrlist.h b/ptrlist.h
> index c5fa4cdd94cb..41d9011c8716 100644
> --- a/ptrlist.h
> +++ b/ptrlist.h
> @@ -12,7 +12,7 @@
>  
>  /* Silly type-safety check ;) */
>  #define CHECK_TYPE(head,ptr)		(void)(&(ptr) == &(head)->list[0])
> -#define TYPEOF(head)			__typeof__(&(head)->list[0])
> +#define TYPEOF(head)			__typeof__((head)->list[0])
>  #define VRFY_PTR_LIST(head)		(void)(sizeof((head)->list[0]))
>  
>  #define LIST_NODE_NR (13)
> @@ -251,7 +251,7 @@ extern void __free_ptr_list(struct ptr_list **);
>  extern void split_ptr_list_head(struct ptr_list *);
>  
>  #define DO_INSERT_CURRENT(new, __head, __list, __nr) do {		\
> -	TYPEOF(__head) __this, __last;					\
> +	TYPEOF(__head) *__this, *__last;				\
>  	if (__list->nr == LIST_NODE_NR) {				\
>  		split_ptr_list_head((struct ptr_list*)__list);		\
>  		if (__nr >= __list->nr) {				\
> @@ -270,8 +270,8 @@ extern void split_ptr_list_head(struct ptr_list *);
>  } while (0)
>  
>  #define DO_DELETE_CURRENT(__head, __list, __nr) do {			\
> -	TYPEOF(__head) __this = __list->list + __nr;			\
> -	TYPEOF(__head) __last = __list->list + __list->nr - 1;		\
> +	TYPEOF(__head) *__this = __list->list + __nr;			\
> +	TYPEOF(__head) *__last = __list->list + __list->nr - 1;		\
>  	while (__this < __last) {					\
>  		__this[0] = __this[1];					\
>  		__this++;						\
> 

[PATCH 2/6] ptrlist: change TYPEOF() into PTRLIST_TYPE()

[Luc Van Oostenryck]

The name of the macro TYPEOF() is too generic and doesn't explain
that it only returns the type of the pointers stored in ptrlists.

So, change he name to something more explicit: PTRLIST_TYPE().

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 ptrlist.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/ptrlist.h b/ptrlist.h
index 41d9011c8716..3b952097545f 100644
--- a/ptrlist.h
+++ b/ptrlist.h
@@ -12,7 +12,7 @@
 
 /* Silly type-safety check ;) */
 #define CHECK_TYPE(head,ptr)		(void)(&(ptr) == &(head)->list[0])
-#define TYPEOF(head)			__typeof__((head)->list[0])
+#define PTRLIST_TYPE(head)		__typeof__((head)->list[0])
 #define VRFY_PTR_LIST(head)		(void)(sizeof((head)->list[0]))
 
 #define LIST_NODE_NR (13)
@@ -75,7 +75,7 @@ extern void __free_ptr_list(struct ptr_list **);
 
 #define ptr_list_nth(lst, nth) ({					\
 		struct ptr_list* head = (struct ptr_list*)(lst);	\
-		(__typeof__((lst)->list[0])) ptr_list_nth_entry(head, nth);\
+		(PTRLIST_TYPE(lst)) ptr_list_nth_entry(head, nth);\
 	})
 
 ////////////////////////////////////////////////////////////////////////
@@ -251,7 +251,7 @@ extern void __free_ptr_list(struct ptr_list **);
 extern void split_ptr_list_head(struct ptr_list *);
 
 #define DO_INSERT_CURRENT(new, __head, __list, __nr) do {		\
-	TYPEOF(__head) *__this, *__last;				\
+	PTRLIST_TYPE(__head) *__this, *__last;				\
 	if (__list->nr == LIST_NODE_NR) {				\
 		split_ptr_list_head((struct ptr_list*)__list);		\
 		if (__nr >= __list->nr) {				\
@@ -270,8 +270,8 @@ extern void split_ptr_list_head(struct ptr_list *);
 } while (0)
 
 #define DO_DELETE_CURRENT(__head, __list, __nr) do {			\
-	TYPEOF(__head) *__this = __list->list + __nr;			\
-	TYPEOF(__head) *__last = __list->list + __list->nr - 1;		\
+	PTRLIST_TYPE(__head) *__this = __list->list + __nr;		\
+	PTRLIST_TYPE(__head) *__last = __list->list + __list->nr - 1;	\
 	while (__this < __last) {					\
 		__this[0] = __this[1];					\
 		__this++;						\
-- 
2.30.0

Re: [PATCH 2/6] ptrlist: change TYPEOF() into PTRLIST_TYPE()

[Ramsay Jones]

On 06/03/2021 10:05, Luc Van Oostenryck wrote:
> The name of the macro TYPEOF() is too generic and doesn't explain
> that it only returns the type of the pointers stored in ptrlists.
> 
> So, change he name to something more explicit: PTRLIST_TYPE().

s/change he/change the/

ATB,
Ramsay Jones

> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
>  ptrlist.h | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/ptrlist.h b/ptrlist.h
> index 41d9011c8716..3b952097545f 100644
> --- a/ptrlist.h
> +++ b/ptrlist.h
> @@ -12,7 +12,7 @@
>  
>  /* Silly type-safety check ;) */
>  #define CHECK_TYPE(head,ptr)		(void)(&(ptr) == &(head)->list[0])
> -#define TYPEOF(head)			__typeof__((head)->list[0])
> +#define PTRLIST_TYPE(head)		__typeof__((head)->list[0])
>  #define VRFY_PTR_LIST(head)		(void)(sizeof((head)->list[0]))
>  
>  #define LIST_NODE_NR (13)
> @@ -75,7 +75,7 @@ extern void __free_ptr_list(struct ptr_list **);
>  
>  #define ptr_list_nth(lst, nth) ({					\
>  		struct ptr_list* head = (struct ptr_list*)(lst);	\
> -		(__typeof__((lst)->list[0])) ptr_list_nth_entry(head, nth);\
> +		(PTRLIST_TYPE(lst)) ptr_list_nth_entry(head, nth);\
>  	})
>  
>  ////////////////////////////////////////////////////////////////////////
> @@ -251,7 +251,7 @@ extern void __free_ptr_list(struct ptr_list **);
>  extern void split_ptr_list_head(struct ptr_list *);
>  
>  #define DO_INSERT_CURRENT(new, __head, __list, __nr) do {		\
> -	TYPEOF(__head) *__this, *__last;				\
> +	PTRLIST_TYPE(__head) *__this, *__last;				\
>  	if (__list->nr == LIST_NODE_NR) {				\
>  		split_ptr_list_head((struct ptr_list*)__list);		\
>  		if (__nr >= __list->nr) {				\
> @@ -270,8 +270,8 @@ extern void split_ptr_list_head(struct ptr_list *);
>  } while (0)
>  
>  #define DO_DELETE_CURRENT(__head, __list, __nr) do {			\
> -	TYPEOF(__head) *__this = __list->list + __nr;			\
> -	TYPEOF(__head) *__last = __list->list + __list->nr - 1;		\
> +	PTRLIST_TYPE(__head) *__this = __list->list + __nr;		\
> +	PTRLIST_TYPE(__head) *__last = __list->list + __list->nr - 1;	\
>  	while (__this < __last) {					\
>  		__this[0] = __this[1];					\
>  		__this++;						\
> 

[PATCH 3/6] ptrlist: add pop_ptr_list()

[Luc Van Oostenryck]

Some algorithms need with a stack or a working list from which the
last element can be removed. The ptrlist API has a function to do
this but it's not typed and thus needs a wrapper for each type
it's used for.

Simplify this by adding a generic (but type-safe) macro for this
while also giving it a nicer name: pop_ptr_list().

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 ptrlist.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/ptrlist.h b/ptrlist.h
index 3b952097545f..0b06142252f5 100644
--- a/ptrlist.h
+++ b/ptrlist.h
@@ -67,6 +67,12 @@ extern void **__add_ptr_list_tag(struct ptr_list **, void *, unsigned long);
 		(__typeof__(&(ptr))) __add_ptr_list_tag(head, ptr, tag);\
 	})
 
+#define pop_ptr_list(l) ({						\
+		PTRLIST_TYPE(*(l)) ptr;					\
+		ptr = delete_ptr_list_last((struct ptr_list**)(l));	\
+		ptr;							\
+	})
+
 extern void __free_ptr_list(struct ptr_list **);
 #define free_ptr_list(list)	do {					\
 		VRFY_PTR_LIST(*(list));					\
-- 
2.30.0

Re: [PATCH 3/6] ptrlist: add pop_ptr_list()

[Ramsay Jones]

On 06/03/2021 10:05, Luc Van Oostenryck wrote:
> Some algorithms need with a stack or a working list from which the

s/with a stack/a stack/

ATB,
Ramsay Jones

> last element can be removed. The ptrlist API has a function to do
> this but it's not typed and thus needs a wrapper for each type
> it's used for.
> 
> Simplify this by adding a generic (but type-safe) macro for this
> while also giving it a nicer name: pop_ptr_list().
> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
>  ptrlist.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/ptrlist.h b/ptrlist.h
> index 3b952097545f..0b06142252f5 100644
> --- a/ptrlist.h
> +++ b/ptrlist.h
> @@ -67,6 +67,12 @@ extern void **__add_ptr_list_tag(struct ptr_list **, void *, unsigned long);
>  		(__typeof__(&(ptr))) __add_ptr_list_tag(head, ptr, tag);\
>  	})
>  
> +#define pop_ptr_list(l) ({						\
> +		PTRLIST_TYPE(*(l)) ptr;					\
> +		ptr = delete_ptr_list_last((struct ptr_list**)(l));	\
> +		ptr;							\
> +	})
> +
>  extern void __free_ptr_list(struct ptr_list **);
>  #define free_ptr_list(list)	do {					\
>  		VRFY_PTR_LIST(*(list));					\
> 

[PATCH 4/6] ptrlist: use ptr_list_nth() instead of linearize_ptr_list()

[Luc Van Oostenryck]

Sparse has a few extra checkers for some functions.
The one for memset has its own helper the retrieve its 3rd arguments.

Remove this helper and use the generic ptr_list_nth() instead.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 sparse.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/sparse.c b/sparse.c
index 151eaf4ef5ed..9d62d4fe4fc4 100644
--- a/sparse.c
+++ b/sparse.c
@@ -163,20 +163,9 @@ static void check_byte_count(struct instruction *insn, pseudo_t count)
 	/* OK, we could try to do the range analysis here */
 }
 
-static pseudo_t argument(struct instruction *call, unsigned int argno)
-{
-	pseudo_t args[8];
-	struct ptr_list *arg_list = (struct ptr_list *) call->arguments;
-
-	argno--;
-	if (linearize_ptr_list(arg_list, (void *)args, 8) > argno)
-		return args[argno];
-	return NULL;
-}
-
 static void check_memset(struct instruction *insn)
 {
-	check_byte_count(insn, argument(insn, 3));
+	check_byte_count(insn, ptr_list_nth(insn->arguments, 3));
 }
 
 #define check_memcpy check_memset
-- 
2.30.0

Re: [PATCH 4/6] ptrlist: use ptr_list_nth() instead of linearize_ptr_list()

[Ramsay Jones]

On 06/03/2021 10:05, Luc Van Oostenryck wrote:
> Sparse has a few extra checkers for some functions.
> The one for memset has its own helper the retrieve its 3rd arguments.

s/the retrieve its 3rd arguments/to retrieve its 3rd argument/

ATB,
Ramsay Jones

> 
> Remove this helper and use the generic ptr_list_nth() instead.
> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
>  sparse.c | 13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/sparse.c b/sparse.c
> index 151eaf4ef5ed..9d62d4fe4fc4 100644
> --- a/sparse.c
> +++ b/sparse.c
> @@ -163,20 +163,9 @@ static void check_byte_count(struct instruction *insn, pseudo_t count)
>  	/* OK, we could try to do the range analysis here */
>  }
>  
> -static pseudo_t argument(struct instruction *call, unsigned int argno)
> -{
> -	pseudo_t args[8];
> -	struct ptr_list *arg_list = (struct ptr_list *) call->arguments;
> -
> -	argno--;
> -	if (linearize_ptr_list(arg_list, (void *)args, 8) > argno)
> -		return args[argno];
> -	return NULL;
> -}
> -
>  static void check_memset(struct instruction *insn)
>  {
> -	check_byte_count(insn, argument(insn, 3));
> +	check_byte_count(insn, ptr_list_nth(insn->arguments, 3));
>  }
>  
>  #define check_memcpy check_memset
> 

[PATCH 5/6] ptrlist: make linearize_ptr_list() generic

[Luc Van Oostenryck]

The ptrlist API has a function to copy the elements of a ptrlist
into an array but it's not typed and thus needs a wrapper (or casts)
for each type it's used for. Also, 'linearize' is confusing since
this is unrelated to Sparse's linearization.

Simplify this by adding a generic (but type-safe) macro for this
(and changing the name): ptr_list_to_array()

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 ptrlist.h  | 6 ++++++
 simplify.c | 4 ++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/ptrlist.h b/ptrlist.h
index 0b06142252f5..5a3dcbeb97ae 100644
--- a/ptrlist.h
+++ b/ptrlist.h
@@ -84,6 +84,12 @@ extern void __free_ptr_list(struct ptr_list **);
 		(PTRLIST_TYPE(lst)) ptr_list_nth_entry(head, nth);\
 	})
 
+#define ptr_list_to_array(list, array, size) ({				\
+		struct ptr_list* head = (struct ptr_list*)(list);	\
+		CHECK_TYPE(list, *array);				\
+		linearize_ptr_list(head, (void**)array, size);		\
+	})
+
 ////////////////////////////////////////////////////////////////////////
 // API
 #define PREPARE_PTR_LIST(head, ptr) \
diff --git a/simplify.c b/simplify.c
index 584078ddca89..cf5b3d748808 100644
--- a/simplify.c
+++ b/simplify.c
@@ -83,7 +83,7 @@ static struct basic_block *phi_parent(struct basic_block *source, pseudo_t pseud
 //	number of element, a positive number if there was
 //	more than expected and a negative one if less.
 //
-// :note: we can't reuse a function like linearize_ptr_list()
+// :note: we can't reuse ptr_list_to_array() for the phi-sources
 //	because any VOIDs in the phi-list must be ignored here
 //	as in this context they mean 'entry has been removed'.
 static int get_phisources(struct instruction *sources[], int nbr, struct instruction *insn)
@@ -116,7 +116,7 @@ static int if_convert_phi(struct instruction *insn)
 	bb = insn->bb;
 	if (get_phisources(array, 2, insn))
 		return 0;
-	if (linearize_ptr_list((struct ptr_list *)bb->parents, (void **)parents, 3) != 2)
+	if (ptr_list_to_array(bb->parents, parents, 3) != 2)
 		return 0;
 	p1 = array[0]->phi_src;
 	bb1 = array[0]->bb;
-- 
2.30.0

Re: [PATCH 5/6] ptrlist: make linearize_ptr_list() generic

[Ramsay Jones]

On 06/03/2021 10:05, Luc Van Oostenryck wrote:
> The ptrlist API has a function to copy the elements of a ptrlist
> into an array but it's not typed and thus needs a wrapper (or casts)
> for each type it's used for. Also, 'linearize' is confusing since
> this is unrelated to Sparse's linearization.
> 
> Simplify this by adding a generic (but type-safe) macro for this
> (and changing the name): ptr_list_to_array()

Hmm, you have removed/renamed all current callers of linearize_ptr_list(),
but you haven't 'changed the name'; you can't, otherwise the new wrapper
macro wouldn't work! ;-)

Maybe, s/and changing the name/with a more descriptive name/

ATB,
Ramsay Jones

> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
>  ptrlist.h  | 6 ++++++
>  simplify.c | 4 ++--
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/ptrlist.h b/ptrlist.h
> index 0b06142252f5..5a3dcbeb97ae 100644
> --- a/ptrlist.h
> +++ b/ptrlist.h
> @@ -84,6 +84,12 @@ extern void __free_ptr_list(struct ptr_list **);
>  		(PTRLIST_TYPE(lst)) ptr_list_nth_entry(head, nth);\
>  	})
>  
> +#define ptr_list_to_array(list, array, size) ({				\
> +		struct ptr_list* head = (struct ptr_list*)(list);	\
> +		CHECK_TYPE(list, *array);				\
> +		linearize_ptr_list(head, (void**)array, size);		\
> +	})
> +
>  ////////////////////////////////////////////////////////////////////////
>  // API
>  #define PREPARE_PTR_LIST(head, ptr) \
> diff --git a/simplify.c b/simplify.c
> index 584078ddca89..cf5b3d748808 100644
> --- a/simplify.c
> +++ b/simplify.c
> @@ -83,7 +83,7 @@ static struct basic_block *phi_parent(struct basic_block *source, pseudo_t pseud
>  //	number of element, a positive number if there was
>  //	more than expected and a negative one if less.
>  //
> -// :note: we can't reuse a function like linearize_ptr_list()
> +// :note: we can't reuse ptr_list_to_array() for the phi-sources
>  //	because any VOIDs in the phi-list must be ignored here
>  //	as in this context they mean 'entry has been removed'.
>  static int get_phisources(struct instruction *sources[], int nbr, struct instruction *insn)
> @@ -116,7 +116,7 @@ static int if_convert_phi(struct instruction *insn)
>  	bb = insn->bb;
>  	if (get_phisources(array, 2, insn))
>  		return 0;
> -	if (linearize_ptr_list((struct ptr_list *)bb->parents, (void **)parents, 3) != 2)
> +	if (ptr_list_to_array(bb->parents, parents, 3) != 2)
>  		return 0;
>  	p1 = array[0]->phi_src;
>  	bb1 = array[0]->bb;
> 

Re: [PATCH 5/6] ptrlist: make linearize_ptr_list() generic

[Luc Van Oostenryck]

On Sat, Mar 06, 2021 at 04:35:52PM +0000, Ramsay Jones wrote:
> On 06/03/2021 10:05, Luc Van Oostenryck wrote:
> > The ptrlist API has a function to copy the elements of a ptrlist
> > into an array but it's not typed and thus needs a wrapper (or casts)
> > for each type it's used for. Also, 'linearize' is confusing since
> > this is unrelated to Sparse's linearization.
> > 
> > Simplify this by adding a generic (but type-safe) macro for this
> > (and changing the name): ptr_list_to_array()
> 
> Hmm, you have removed/renamed all current callers of linearize_ptr_list(),
> but you haven't 'changed the name'; you can't, otherwise the new wrapper
> macro wouldn't work! ;-)

I've changed the name of the interface. But yes, the message was confusing.

> Maybe, s/and changing the name/with a more descriptive name/

Yes, this is better. Thank you.
-- Luc

[PATCH 6/6] ptrlist: change return value of linearize_ptr_list()/ptr_list_to_array()

[Luc Van Oostenryck]

The function linearize_ptr_list() is annoying to use because it
returns the number of elements put in the array. So, if you need
to know if the list contained the expected number of entries,
you need to allocate to array with one more entries and check
that the return value is one less than the maximum size.

Change this, so that this function returns the total number of
entries in the list, much like it's done for snprintf().

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 ptrlist.c  | 10 +++++-----
 simplify.c |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/ptrlist.c b/ptrlist.c
index 0f0b3f6d818f..ecfbc07b2b6d 100644
--- a/ptrlist.c
+++ b/ptrlist.c
@@ -154,10 +154,10 @@ void *ptr_list_nth_entry(struct ptr_list *list, unsigned int idx)
 // @head: the list to be linearized
 // @arr: a ``void*`` array to fill with @head's entries
 // @max: the maximum number of entries to store into @arr
-// @return: the number of entries linearized.
+// @return: the number of entries in the list.
 //
 // Linearize the entries of a list up to a total of @max,
-// and return the nr of entries linearized.
+// and return the nunmber of entries in the list.
 //
 // The array to linearize into (@arr) should really
 // be ``void *x[]``, but we want to let people fill in any kind
@@ -170,14 +170,14 @@ int linearize_ptr_list(struct ptr_list *head, void **arr, int max)
 
 		do {
 			int i = list->nr;
+			nr += i;
+			if (max == 0)
+				continue;
 			if (i > max) 
 				i = max;
 			memcpy(arr, list->list, i*sizeof(void *));
 			arr += i;
-			nr += i;
 			max -= i;
-			if (!max)
-				break;
 		} while ((list = list->next) != head);
 	}
 	return nr;
diff --git a/simplify.c b/simplify.c
index cf5b3d748808..207af8edf28f 100644
--- a/simplify.c
+++ b/simplify.c
@@ -108,7 +108,7 @@ static int get_phisources(struct instruction *sources[], int nbr, struct instruc
 static int if_convert_phi(struct instruction *insn)
 {
 	struct instruction *array[2];
-	struct basic_block *parents[3];
+	struct basic_block *parents[2];
 	struct basic_block *bb, *bb1, *bb2, *source;
 	struct instruction *br;
 	pseudo_t p1, p2;
@@ -116,7 +116,7 @@ static int if_convert_phi(struct instruction *insn)
 	bb = insn->bb;
 	if (get_phisources(array, 2, insn))
 		return 0;
-	if (ptr_list_to_array(bb->parents, parents, 3) != 2)
+	if (ptr_list_to_array(bb->parents, parents, 2) != 2)
 		return 0;
 	p1 = array[0]->phi_src;
 	bb1 = array[0]->bb;
-- 
2.30.0

Re: [PATCH 6/6] ptrlist: change return value of linearize_ptr_list()/ptr_list_to_array()

[Ramsay Jones]

On 06/03/2021 10:05, Luc Van Oostenryck wrote:
> The function linearize_ptr_list() is annoying to use because it
> returns the number of elements put in the array. So, if you need
> to know if the list contained the expected number of entries,
> you need to allocate to array with one more entries and check

s/allocate to array/allocate an array/

> that the return value is one less than the maximum size.
> 
> Change this, so that this function returns the total number of
> entries in the list, much like it's done for snprintf().

But this requires setting max == 0, right? This isn't documented.

> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
>  ptrlist.c  | 10 +++++-----
>  simplify.c |  4 ++--
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/ptrlist.c b/ptrlist.c
> index 0f0b3f6d818f..ecfbc07b2b6d 100644
> --- a/ptrlist.c
> +++ b/ptrlist.c
> @@ -154,10 +154,10 @@ void *ptr_list_nth_entry(struct ptr_list *list, unsigned int idx)
>  // @head: the list to be linearized
>  // @arr: a ``void*`` array to fill with @head's entries
>  // @max: the maximum number of entries to store into @arr
> -// @return: the number of entries linearized.
> +// @return: the number of entries in the list.
>  //
>  // Linearize the entries of a list up to a total of @max,
> -// and return the nr of entries linearized.
> +// and return the nunmber of entries in the list.

s/nunmber/number/

Somewhere here, the behaviour with max == 0 on input needs to
be documented.

ATB,
Ramsay Jones

>  //
>  // The array to linearize into (@arr) should really
>  // be ``void *x[]``, but we want to let people fill in any kind
> @@ -170,14 +170,14 @@ int linearize_ptr_list(struct ptr_list *head, void **arr, int max)
>  
>  		do {
>  			int i = list->nr;
> +			nr += i;
> +			if (max == 0)
> +				continue;
>  			if (i > max) 
>  				i = max;
>  			memcpy(arr, list->list, i*sizeof(void *));
>  			arr += i;
> -			nr += i;
>  			max -= i;
> -			if (!max)
> -				break;
>  		} while ((list = list->next) != head);
>  	}
>  	return nr;
> diff --git a/simplify.c b/simplify.c
> index cf5b3d748808..207af8edf28f 100644
> --- a/simplify.c
> +++ b/simplify.c
> @@ -108,7 +108,7 @@ static int get_phisources(struct instruction *sources[], int nbr, struct instruc
>  static int if_convert_phi(struct instruction *insn)
>  {
>  	struct instruction *array[2];
> -	struct basic_block *parents[3];
> +	struct basic_block *parents[2];
>  	struct basic_block *bb, *bb1, *bb2, *source;
>  	struct instruction *br;
>  	pseudo_t p1, p2;
> @@ -116,7 +116,7 @@ static int if_convert_phi(struct instruction *insn)
>  	bb = insn->bb;
>  	if (get_phisources(array, 2, insn))
>  		return 0;
> -	if (ptr_list_to_array(bb->parents, parents, 3) != 2)
> +	if (ptr_list_to_array(bb->parents, parents, 2) != 2)
>  		return 0;
>  	p1 = array[0]->phi_src;
>  	bb1 = array[0]->bb;
> 

Re: [PATCH 6/6] ptrlist: change return value of linearize_ptr_list()/ptr_list_to_array()

[Luc Van Oostenryck]

On Sat, Mar 06, 2021 at 04:43:21PM +0000, Ramsay Jones wrote:
> On 06/03/2021 10:05, Luc Van Oostenryck wrote:
> > Change this, so that this function returns the total number of
> > entries in the list, much like it's done for snprintf().
> 
> But this requires setting max == 0, right? This isn't documented.

No no, there is nothing special with max == 0 at the interface level.
If the list contains 3 elements but you're only interested in the cases
it contains 2, you now call:
	... array[2];
	int nbr = linearize_ptr_list(list, array, 2);
and it'll only fill 2 elements but it will return 3, so can you write:
	if (nbr == 2) {
		... do stuff ...
	}
IOW, it returns the number of elements that would have been written
to the array if 'max' would have been infinite.

Previously, it would have returned 2 because the return value was
capped to 'max' / it returned he number of elements written. So you
had no idea if the list effectively contained only 2 elements of if
there was some more and so you had to call it with one extra element
to check this:
	... array[3];
	int nbr = linearize_ptr_list(list, array, 3);
	if (nbr == 2) {
		...
	}

Thanks for noticing all these typos,
-- Luc