[PATCH 0/2] fix rem_usage()

[PATCH 0/2] fix rem_usage()

[Luc Van Oostenryck]

These two patches fix a bug with rem_usage(), after having removed the
last element of a def-use list, tries to kill the defining instruction
although only PSEUDO_REGs and PSEUDO_PHIs have one.


Luc Van Oostenryck (2):
  add helper has_definition()
  fix rem_usage()

 linearize.h | 5 +++++
 simplify.c  | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

-- 
2.29.2

[PATCH 1/2] add helper has_definition()

[Luc Van Oostenryck]

Add he helper has_definition() to check if the pseudo belong to one
of the pseudo types having a definition: PSEUDO_REG & PSEUDO_PHI.

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

diff --git a/linearize.h b/linearize.h
index 2c548d43526f..c5bdd04257a1 100644
--- a/linearize.h
+++ b/linearize.h
@@ -249,6 +249,11 @@ static inline int has_use_list(pseudo_t p)
 	return (p && p->type != PSEUDO_VOID && p->type != PSEUDO_UNDEF && p->type != PSEUDO_VAL);
 }
 
+static inline bool has_definition(pseudo_t p)
+{
+	return p->type == PSEUDO_REG || p->type == PSEUDO_PHI;
+}
+
 static inline int pseudo_user_list_size(struct pseudo_user_list *list)
 {
 	return ptr_list_size((struct ptr_list *)list);
-- 
2.29.2

Re: [PATCH 1/2] add helper has_definition()

[Ramsay Jones]

On 27/12/2020 09:27, Luc Van Oostenryck wrote:
> Add he helper has_definition() to check if the pseudo belong to one

s/Add he/Add the/

ATB,
Ramsay Jones

> of the pseudo types having a definition: PSEUDO_REG & PSEUDO_PHI.
> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
>  linearize.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/linearize.h b/linearize.h
> index 2c548d43526f..c5bdd04257a1 100644
> --- a/linearize.h
> +++ b/linearize.h
> @@ -249,6 +249,11 @@ static inline int has_use_list(pseudo_t p)
>  	return (p && p->type != PSEUDO_VOID && p->type != PSEUDO_UNDEF && p->type != PSEUDO_VAL);
>  }
>  
> +static inline bool has_definition(pseudo_t p)
> +{
> +	return p->type == PSEUDO_REG || p->type == PSEUDO_PHI;
> +}
> +
>  static inline int pseudo_user_list_size(struct pseudo_user_list *list)
>  {
>  	return ptr_list_size((struct ptr_list *)list);
> 

[PATCH 2/2] fix rem_usage()

[Luc Van Oostenryck]

rem_usage() is used to remove an element from a def-use chain. Optionally,
if the chain become empty, the defining instruction can also be killed.

This optional part is currently be done on all pseudos but only those
having a definition should be concerned.

Fix this by adding a check so that only PSEUDO_REGs and PSEUDO_PHIs are killed.

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

diff --git a/simplify.c b/simplify.c
index a877b693b610..12482d787189 100644
--- a/simplify.c
+++ b/simplify.c
@@ -271,7 +271,7 @@ static inline void rem_usage(pseudo_t p, pseudo_t *usep, int kill)
 {
 	if (has_use_list(p)) {
 		delete_pseudo_user_list_entry(&p->users, usep, 1);
-		if (kill && !p->users)
+		if (kill && !p->users && has_definition(p))
 			kill_instruction(p->def);
 	}
 }
-- 
2.29.2

Re: [PATCH 2/2] fix rem_usage()

[Ramsay Jones]

On 27/12/2020 09:27, Luc Van Oostenryck wrote:
> rem_usage() is used to remove an element from a def-use chain. Optionally,

Hmm, rename this to 'remove_usage()' at the same time?

ATB,
Ramsay Jones

> if the chain become empty, the defining instruction can also be killed.
> 
> This optional part is currently be done on all pseudos but only those
> having a definition should be concerned.
> 
> Fix this by adding a check so that only PSEUDO_REGs and PSEUDO_PHIs are killed.
> 
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
>  simplify.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/simplify.c b/simplify.c
> index a877b693b610..12482d787189 100644
> --- a/simplify.c
> +++ b/simplify.c
> @@ -271,7 +271,7 @@ static inline void rem_usage(pseudo_t p, pseudo_t *usep, int kill)
>  {
>  	if (has_use_list(p)) {
>  		delete_pseudo_user_list_entry(&p->users, usep, 1);
> -		if (kill && !p->users)
> +		if (kill && !p->users && has_definition(p))
>  			kill_instruction(p->def);
>  	}
>  }
> 

Re: [PATCH 2/2] fix rem_usage()

[Luc Van Oostenryck]

On Mon, Dec 28, 2020 at 05:22:36PM +0000, Ramsay Jones wrote:
> 
> 
> On 27/12/2020 09:27, Luc Van Oostenryck wrote:
> > rem_usage() is used to remove an element from a def-use chain. Optionally,
> 
> Hmm, rename this to 'remove_usage()' at the same time?

It can't, 'remove_usage()' is already taken and 'rem_usage()' is more
for internal uses than 'remove_usage()'.

-- Luc

Re: [PATCH 2/2] fix rem_usage()

[Ramsay Jones]

On 28/12/2020 21:39, Luc Van Oostenryck wrote:
> On Mon, Dec 28, 2020 at 05:22:36PM +0000, Ramsay Jones wrote:
>>
>>
>> On 27/12/2020 09:27, Luc Van Oostenryck wrote:
>>> rem_usage() is used to remove an element from a def-use chain. Optionally,
>>
>> Hmm, rename this to 'remove_usage()' at the same time?
> 
> It can't, 'remove_usage()' is already taken and 'rem_usage()' is more
> for internal uses than 'remove_usage()'.

Ugh! I just took a quick look. Ugly. ;-)

Well, I think I would rename it 'remove_use()' or somesuch instead.

ATB,
Ramsay Jones

Re: [PATCH 2/2] fix rem_usage()

[Ramsay Jones]

On 28/12/2020 22:09, Ramsay Jones wrote:
> 
> 
> On 28/12/2020 21:39, Luc Van Oostenryck wrote:
>> On Mon, Dec 28, 2020 at 05:22:36PM +0000, Ramsay Jones wrote:
>>>
>>>
>>> On 27/12/2020 09:27, Luc Van Oostenryck wrote:
>>>> rem_usage() is used to remove an element from a def-use chain. Optionally,
>>>
>>> Hmm, rename this to 'remove_usage()' at the same time?
>>
>> It can't, 'remove_usage()' is already taken and 'rem_usage()' is more
>> for internal uses than 'remove_usage()'.
> 
> Ugh! I just took a quick look. Ugly. ;-)
> 
> Well, I think I would rename it 'remove_use()' or somesuch instead.

Ahem, except you can't, because it is already used ... (facepalm).

Ho Hum. ;-)

ATB,
Ramsay Jones