linux-sparse.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] fix expansion cost of pure functions
@ 2017-02-16 15:45 Luc Van Oostenryck
  2017-02-22 23:57 ` Christopher Li
  0 siblings, 1 reply; 5+ messages in thread
From: Luc Van Oostenryck @ 2017-02-16 15:45 UTC (permalink / raw)
  To: linux-sparse; +Cc: Christopher Li, Luc Van Oostenryck

Expansion cost of an expression should be a monotonically
increasing function of its sub-expressions.

Here, for the costs of calling a pure function, the costs is
reset to zero (wich is used when the expression expand to
a constant/to test if the expression is a constant or not).

Fix this by setting the cost as the total expansion cost of
all the arguments plus one for the function itself.

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

diff --git a/expand.c b/expand.c
index 7af12707e..738c79c51 100644
--- a/expand.c
+++ b/expand.c
@@ -796,7 +796,7 @@ static int expand_symbol_call(struct expression *expr, int cost)
 		return ctype->op->expand(expr, cost);
 
 	if (ctype->ctype.modifiers & MOD_PURE)
-		return 0;
+		return cost + 1;
 
 	return SIDE_EFFECTS;
 }
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] fix expansion cost of pure functions
  2017-02-16 15:45 [PATCH] fix expansion cost of pure functions Luc Van Oostenryck
@ 2017-02-22 23:57 ` Christopher Li
  2017-02-23  1:01   ` Linus Torvalds
  0 siblings, 1 reply; 5+ messages in thread
From: Christopher Li @ 2017-02-22 23:57 UTC (permalink / raw)
  To: Luc Van Oostenryck; +Cc: Linux-Sparse

On Thu, Feb 16, 2017 at 11:45 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> Expansion cost of an expression should be a monotonically
> increasing function of its sub-expressions.
>
> Here, for the costs of calling a pure function, the costs is
> reset to zero (wich is used when the expression expand to
> a constant/to test if the expression is a constant or not).
>
> Fix this by setting the cost as the total expansion cost of
> all the arguments plus one for the function itself.

I am not sure about this one, adding CC to Linus.

Currently pure function and constant function both share MOD_PURE.
If the function has attribute "pure" or "constant", then the user is
telling the compiler this function does not have side effects.

I think dropping to zero has some thing to do with get rid of
the side effect cost. So it possible the cost can be dropping.
But maybe it shouldn't drop to zero.

Chris

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fix expansion cost of pure functions
  2017-02-22 23:57 ` Christopher Li
@ 2017-02-23  1:01   ` Linus Torvalds
  2017-02-23  9:55     ` Christopher Li
  2017-02-23  9:56     ` Christopher Li
  0 siblings, 2 replies; 5+ messages in thread
From: Linus Torvalds @ 2017-02-23  1:01 UTC (permalink / raw)
  To: Christopher Li; +Cc: Luc Van Oostenryck, Linux-Sparse

On Wed, Feb 22, 2017 at 3:57 PM, Christopher Li <sparse@chrisli.org> wrote:
>
> I think dropping to zero has some thing to do with get rid of
> the side effect cost. So it possible the cost can be dropping.

No, I think the zero is just a bug, introduced in commit 8376ab091a
("sparse: Fix __builtin_safe_p for pure and const functions").

I might be missing something, but I think Luc's patch is fine.

              Linus

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fix expansion cost of pure functions
  2017-02-23  1:01   ` Linus Torvalds
@ 2017-02-23  9:55     ` Christopher Li
  2017-02-23  9:56     ` Christopher Li
  1 sibling, 0 replies; 5+ messages in thread
From: Christopher Li @ 2017-02-23  9:55 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Luc Van Oostenryck, Linux-Sparse

On Thu, Feb 23, 2017 at 9:01 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Wed, Feb 22, 2017 at 3:57 PM, Christopher Li <sparse@chrisli.org> wrote:
>>
>> I think dropping to zero has some thing to do with get rid of
>> the side effect cost. So it possible the cost can be dropping.
>
> No, I think the zero is just a bug, introduced in commit 8376ab091a
> ("sparse: Fix __builtin_safe_p for pure and const functions").
>
> I might be missing something, but I think Luc's patch is fine.
>
>               Linus

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fix expansion cost of pure functions
  2017-02-23  1:01   ` Linus Torvalds
  2017-02-23  9:55     ` Christopher Li
@ 2017-02-23  9:56     ` Christopher Li
  1 sibling, 0 replies; 5+ messages in thread
From: Christopher Li @ 2017-02-23  9:56 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Luc Van Oostenryck, Linux-Sparse

On Thu, Feb 23, 2017 at 9:01 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> No, I think the zero is just a bug, introduced in commit 8376ab091a
> ("sparse: Fix __builtin_safe_p for pure and const functions").
>
> I might be missing something, but I think Luc's patch is fine.

Thanks for the insight. I will apply it.

Chris

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-02-23  9:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-16 15:45 [PATCH] fix expansion cost of pure functions Luc Van Oostenryck
2017-02-22 23:57 ` Christopher Li
2017-02-23  1:01   ` Linus Torvalds
2017-02-23  9:55     ` Christopher Li
2017-02-23  9:56     ` Christopher Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).