public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Add divide by zero side effect.
@ 2022-05-17 18:39 Andrew MacLeod
  2022-05-18  6:28 ` Richard Biener
                   ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: Andrew MacLeod @ 2022-05-17 18:39 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 592 bytes --]

I haven't checked this patch in yet.  This implements a side effect that 
the divisor cannot be 0 after a divide executes. This allows us to fold 
the divide away:

a = b / c;
if (c == 0)
   dead();

This bootstraps on x86_64-pc-linux-gnu with no regressions, but I first 
wanted to check to see if there are some flags or conditions that should 
e checked in order NOT to do this optimization.  I am guessing there is 
probably something :-)    Anyway, this is how we straightforwardly add 
side effects now.

Does the patch conditions need tweaking to apply the side effect?

Andrew


[-- Attachment #2: zero.diff --]
[-- Type: text/x-patch, Size: 1950 bytes --]

commit 3bbcccf2ddd4d50cc5febf630bd8b55a45688352
Author: Andrew MacLeod <amacleod@redhat.com>
Date:   Mon Apr 4 16:13:57 2022 -0400

    Add divide by zero side effect.
    
    After a divide, we know the divisor is not zero.
    
            gcc/
            * gimple-range-side-effect.cc (stmt_side_effects::stmt_side_effects):
            Add support for all divides.
    
            gcc/testsuite/
            * gcc.dg/tree-ssa/evrp-zero.c: New.

diff --git a/gcc/gimple-range-side-effect.cc b/gcc/gimple-range-side-effect.cc
index 2c8c77dc569..548e4bea313 100644
--- a/gcc/gimple-range-side-effect.cc
+++ b/gcc/gimple-range-side-effect.cc
@@ -116,6 +116,23 @@ stmt_side_effects::stmt_side_effects (gimple *s)
     walk_stmt_load_store_ops (s, (void *)this, non_null_loadstore,
 			      non_null_loadstore);
 
+  if (is_a<gassign *> (s))
+    {
+      switch (gimple_assign_rhs_code (s))
+	{
+	case TRUNC_DIV_EXPR:
+	case CEIL_DIV_EXPR:
+	case FLOOR_DIV_EXPR:
+	case ROUND_DIV_EXPR:
+	case EXACT_DIV_EXPR:
+	  // Divide means operand 2 is not zero after this stmt.
+	  if (gimple_range_ssa_p (gimple_assign_rhs2 (s)))
+	    add_nonzero (gimple_assign_rhs2 (s));
+	  break;
+	default:
+	  break;
+	}
+    }
 }
 
 // -------------------------------------------------------------------------
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/evrp-zero.c b/gcc/testsuite/gcc.dg/tree-ssa/evrp-zero.c
new file mode 100644
index 00000000000..2b76e449c9b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/evrp-zero.c
@@ -0,0 +1,28 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-evrp " } */
+
+/* Side effects of divide are that the divisor cannot be 0. */
+
+void dead (int);
+
+void
+f1 (int a, int c) {
+  int b = a;
+  if (a / c > 10)
+    b = c;
+
+  if (c == 0)
+    dead (b);
+}
+
+
+void
+f2 (int a, int c) {
+  int nz = c == 0;
+  int b = a / c;
+  if (nz)
+    dead (0);
+}
+
+
+/* { dg-final { scan-tree-dump-not "dead" "evrp" } } */

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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-17 18:39 [PATCH] Add divide by zero side effect Andrew MacLeod
@ 2022-05-18  6:28 ` Richard Biener
  2022-05-18 13:20   ` Andrew MacLeod
  2022-05-18 18:13 ` Segher Boessenkool
  2022-05-27 19:33 ` Andi Kleen
  2 siblings, 1 reply; 26+ messages in thread
From: Richard Biener @ 2022-05-18  6:28 UTC (permalink / raw)
  To: Andrew MacLeod; +Cc: gcc-patches

On Tue, May 17, 2022 at 8:40 PM Andrew MacLeod via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> I haven't checked this patch in yet.  This implements a side effect that
> the divisor cannot be 0 after a divide executes. This allows us to fold
> the divide away:
>
> a = b / c;
> if (c == 0)
>    dead();
>
> This bootstraps on x86_64-pc-linux-gnu with no regressions, but I first
> wanted to check to see if there are some flags or conditions that should
> e checked in order NOT to do this optimization.  I am guessing there is
> probably something :-)    Anyway, this is how we straightforwardly add
> side effects now.
>
> Does the patch conditions need tweaking to apply the side effect?

What does "after the stmt" mean?  If the stmt throws internally then on
the EH edge the divisor can be zero.

How do you fold away the divide in your above example?

Richard.

>
> Andrew
>

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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-18  6:28 ` Richard Biener
@ 2022-05-18 13:20   ` Andrew MacLeod
  0 siblings, 0 replies; 26+ messages in thread
From: Andrew MacLeod @ 2022-05-18 13:20 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

On 5/18/22 02:28, Richard Biener wrote:
> On Tue, May 17, 2022 at 8:40 PM Andrew MacLeod via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>> I haven't checked this patch in yet.  This implements a side effect that
>> the divisor cannot be 0 after a divide executes. This allows us to fold
>> the divide away:
>>
>> a = b / c;
>> if (c == 0)
>>     dead();
>>
>> This bootstraps on x86_64-pc-linux-gnu with no regressions, but I first
>> wanted to check to see if there are some flags or conditions that should
>> e checked in order NOT to do this optimization.  I am guessing there is
>> probably something :-)    Anyway, this is how we straightforwardly add
>> side effects now.
>>
>> Does the patch conditions need tweaking to apply the side effect?
> What does "after the stmt" mean?  If the stmt throws internally then on
> the EH edge the divisor can be zero.
>
> How do you fold away the divide in your above example?
>
We dont fold anyway the divide, just the condition checking if c == 0.. 
this would be identical in function to

b= *ptr;
if (ptr == 0)
   dead();

after the b = *ptr stmt is done, ptr is known to be non-zero. likewise,  
after a = b / c, all subsequent stmts know c is non-zero

Any outgoing EH edge from the block will not have this side effect 
applied (ie ptr and c will still be varying, or whatever they were to 
start).  All other edges will have the non-zero effect, as will any 
stmts in this block which occur after the initial one that triggers the 
side effect.

Andrew



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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-17 18:39 [PATCH] Add divide by zero side effect Andrew MacLeod
  2022-05-18  6:28 ` Richard Biener
@ 2022-05-18 18:13 ` Segher Boessenkool
  2022-05-18 20:24   ` Andrew MacLeod
  2022-05-27 19:33 ` Andi Kleen
  2 siblings, 1 reply; 26+ messages in thread
From: Segher Boessenkool @ 2022-05-18 18:13 UTC (permalink / raw)
  To: Andrew MacLeod; +Cc: gcc-patches

On Tue, May 17, 2022 at 02:39:11PM -0400, Andrew MacLeod via Gcc-patches wrote:
> I haven't checked this patch in yet.  This implements a side effect that 
> the divisor cannot be 0 after a divide executes. This allows us to fold 
> the divide away:

"Side effect" already has a meaning, very commonly used in language
theory, and even in the C standard itself: a function has a side effect
if it does something more than just return a value: if it changes state.
This can be some I/O, or it can just be writing to some non-local data.

Side effects are crucial to what a compiler does, and they are used all
over the place (the gcc/ dir has some thousand mentions of it for
example).

Please don't make life hard for everyone by overloading this term.


Segher

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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-18 18:13 ` Segher Boessenkool
@ 2022-05-18 20:24   ` Andrew MacLeod
  2022-05-18 20:40     ` Segher Boessenkool
  2022-05-20 12:09     ` Alexandre Oliva
  0 siblings, 2 replies; 26+ messages in thread
From: Andrew MacLeod @ 2022-05-18 20:24 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: gcc-patches

On 5/18/22 14:13, Segher Boessenkool wrote:
> On Tue, May 17, 2022 at 02:39:11PM -0400, Andrew MacLeod via Gcc-patches wrote:
>> I haven't checked this patch in yet.  This implements a side effect that
>> the divisor cannot be 0 after a divide executes. This allows us to fold
>> the divide away:
> "Side effect" already has a meaning, very commonly used in language
> theory, and even in the C standard itself: a function has a side effect
> if it does something more than just return a value: if it changes state.
> This can be some I/O, or it can just be writing to some non-local data.
>
> Side effects are crucial to what a compiler does, and they are used all
> over the place (the gcc/ dir has some thousand mentions of it for
> example).
>
> Please don't make life hard for everyone by overloading this term.
>
I'm open to suggestions for a better term!

Is there a commonly used alternate term to describe an observable effect 
on the value of an input operand?

Andrew



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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-18 20:24   ` Andrew MacLeod
@ 2022-05-18 20:40     ` Segher Boessenkool
  2022-05-19 13:22       ` Andrew MacLeod
  2022-05-20 12:09     ` Alexandre Oliva
  1 sibling, 1 reply; 26+ messages in thread
From: Segher Boessenkool @ 2022-05-18 20:40 UTC (permalink / raw)
  To: Andrew MacLeod; +Cc: gcc-patches

On Wed, May 18, 2022 at 04:24:06PM -0400, Andrew MacLeod wrote:
> On 5/18/22 14:13, Segher Boessenkool wrote:
> >"Side effect" already has a meaning, very commonly used in language
> >theory, and even in the C standard itself: a function has a side effect
> >if it does something more than just return a value: if it changes state.
> >This can be some I/O, or it can just be writing to some non-local data.
> >
> >Side effects are crucial to what a compiler does, and they are used all
> >over the place (the gcc/ dir has some thousand mentions of it for
> >example).
> >
> >Please don't make life hard for everyone by overloading this term.
> >
> I'm open to suggestions for a better term!

Glad to hear that, and this isn't set in stione yet!

> Is there a commonly used alternate term to describe an observable effect 
> on the value of an input operand?

I'd use something with "known" in the name.  But:

As far as I understand what you are doing this is not an effect on the
operand at all!  It cannot be one even, the operand is an input only
after all.  Instead, it changes what is known about the value of that
input: it cannot be 0 in this case, it is known to not be 0.

This is similar to other known value things we have in GCC already.  Can
you not just use one of those, even?  What are the benefit to this new
abstraction?


Segher

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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-18 20:40     ` Segher Boessenkool
@ 2022-05-19 13:22       ` Andrew MacLeod
  2022-05-19 22:23         ` Segher Boessenkool
  0 siblings, 1 reply; 26+ messages in thread
From: Andrew MacLeod @ 2022-05-19 13:22 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: gcc-patches

On 5/18/22 16:40, Segher Boessenkool wrote:
> On Wed, May 18, 2022 at 04:24:06PM -0400, Andrew MacLeod wrote:
>> On 5/18/22 14:13, Segher Boessenkool wrote:
>>> "Side effect" already has a meaning, very commonly used in language
>>> theory, and even in the C standard itself: a function has a side effect
>>> if it does something more than just return a value: if it changes state.
>>> This can be some I/O, or it can just be writing to some non-local data.
>>>
>>> Side effects are crucial to what a compiler does, and they are used all
>>> over the place (the gcc/ dir has some thousand mentions of it for
>>> example).
>>>
>>> Please don't make life hard for everyone by overloading this term.
>>>
>> I'm open to suggestions for a better term!
> Glad to hear that, and this isn't set in stione yet!
>
>> Is there a commonly used alternate term to describe an observable effect
>> on the value of an input operand?
> I'd use something with "known" in the name.  But:
>
> As far as I understand what you are doing this is not an effect on the
> operand at all!  It cannot be one even, the operand is an input only
> after all.  Instead, it changes what is known about the value of that
> input: it cannot be 0 in this case, it is known to not be 0.
>
> This is similar to other known value things we have in GCC already.  Can
> you not just use one of those, even?  What are the benefit to this new
> abstraction?

Well, This is a component of ranger tracking value ranges..  it is 
recording the "side-effect" of the stmt on the known range of an object. 
The file is called  "gimple-range-side-effect.h"

Its a generalization of how ranger tracks non-null pointer values, 
enabling it to track arbitrary observable ranges values. (The old 
mechanism also utilized immediate-use chains, which prevented it from 
being utilized in gimple-folding)

WIth this change, we can also track things like a = b / c causing the 
effect that c is known non-zero after the statement if there were no 
traps, or https://gcc.gnu.org/bugzilla/show_bug.cgi?id=31178 , which 
after 15 years, we can now simply indicate that for  a = b >> c , its 
only defined behaviour if c is in the range [0, PRECISION(b)]

So its basically just a generalization of how we track known values 
within the range system.

Andrew



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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-19 13:22       ` Andrew MacLeod
@ 2022-05-19 22:23         ` Segher Boessenkool
  2022-05-20  2:14           ` Andrew MacLeod
  0 siblings, 1 reply; 26+ messages in thread
From: Segher Boessenkool @ 2022-05-19 22:23 UTC (permalink / raw)
  To: Andrew MacLeod; +Cc: gcc-patches

On Thu, May 19, 2022 at 09:22:32AM -0400, Andrew MacLeod wrote:
> On 5/18/22 16:40, Segher Boessenkool wrote:
> >On Wed, May 18, 2022 at 04:24:06PM -0400, Andrew MacLeod wrote:
> >>On 5/18/22 14:13, Segher Boessenkool wrote:
> >>>"Side effect" already has a meaning, very commonly used in language
> >>>theory, and even in the C standard itself: a function has a side effect
> >>>if it does something more than just return a value: if it changes state.
> >>>This can be some I/O, or it can just be writing to some non-local data.
> >>>
> >>>Side effects are crucial to what a compiler does, and they are used all
> >>>over the place (the gcc/ dir has some thousand mentions of it for
> >>>example).
> >>>
> >>>Please don't make life hard for everyone by overloading this term.
> >>>
> >>I'm open to suggestions for a better term!
> >Glad to hear that, and this isn't set in stione yet!
> >
> >>Is there a commonly used alternate term to describe an observable effect
> >>on the value of an input operand?
> >I'd use something with "known" in the name.  But:
> >
> >As far as I understand what you are doing this is not an effect on the
> >operand at all!  It cannot be one even, the operand is an input only
> >after all.  Instead, it changes what is known about the value of that
> >input: it cannot be 0 in this case, it is known to not be 0.
> >
> >This is similar to other known value things we have in GCC already.  Can
> >you not just use one of those, even?  What are the benefit to this new
> >abstraction?
> 
> Well, This is a component of ranger tracking value ranges..  it is 
> recording the "side-effect" of the stmt on the known range of an object. 
> The file is called  "gimple-range-side-effect.h"

So the file name is confusingly wrong as well.

> Its a generalization of how ranger tracks non-null pointer values, 
> enabling it to track arbitrary observable ranges values. (The old 
> mechanism also utilized immediate-use chains, which prevented it from 
> being utilized in gimple-folding)
> 
> WIth this change, we can also track things like a = b / c causing the 
> effect that c is known non-zero after the statement if there were no 
> traps, or https://gcc.gnu.org/bugzilla/show_bug.cgi?id=31178 , which 
> after 15 years, we can now simply indicate that for  a = b >> c , its 
> only defined behaviour if c is in the range [0, PRECISION(b)]
> 
> So its basically just a generalization of how we track known values 
> within the range system.

Sure.  Just the name is harmful :-(


Segher

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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-19 22:23         ` Segher Boessenkool
@ 2022-05-20  2:14           ` Andrew MacLeod
  2022-05-20  6:25             ` Richard Biener
  0 siblings, 1 reply; 26+ messages in thread
From: Andrew MacLeod @ 2022-05-20  2:14 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: gcc-patches

On 5/19/22 18:23, Segher Boessenkool wrote:
> On Thu, May 19, 2022 at 09:22:32AM -0400, Andrew MacLeod wrote:
>> On 5/18/22 16:40, Segher Boessenkool wrote:
>>> On Wed, May 18, 2022 at 04:24:06PM -0400, Andrew MacLeod wrote:
>>>> On 5/18/22 14:13, Segher Boessenkool wrote:
>>>>> "Side effect" already has a meaning, very commonly used in language
>>>>> theory, and even in the C standard itself: a function has a side effect
>>>>> if it does something more than just return a value: if it changes state.
>>>>> This can be some I/O, or it can just be writing to some non-local data.
>>>>>
>>>>> Side effects are crucial to what a compiler does, and they are used all
>>>>> over the place (the gcc/ dir has some thousand mentions of it for
>>>>> example).
>>>>>
>>>>> Please don't make life hard for everyone by overloading this term.
>>>>>
>>>> I'm open to suggestions for a better term!
>>> Glad to hear that, and this isn't set in stione yet!
>>>
>>>> Is there a commonly used alternate term to describe an observable effect
>>>> on the value of an input operand?
>>> I'd use something with "known" in the name.  But:
>>>
>>> As far as I understand what you are doing this is not an effect on the
>>> operand at all!  It cannot be one even, the operand is an input only
>>> after all.  Instead, it changes what is known about the value of that
>>> input: it cannot be 0 in this case, it is known to not be 0.
>>>
>>> This is similar to other known value things we have in GCC already.  Can
>>> you not just use one of those, even?  What are the benefit to this new
>>> abstraction?
>> Well, This is a component of ranger tracking value ranges..  it is
>> recording the "side-effect" of the stmt on the known range of an object.
>> The file is called  "gimple-range-side-effect.h"
> So the file name is confusingly wrong as well.
>
>> Its a generalization of how ranger tracks non-null pointer values,
>> enabling it to track arbitrary observable ranges values. (The old
>> mechanism also utilized immediate-use chains, which prevented it from
>> being utilized in gimple-folding)
>>
>> WIth this change, we can also track things like a = b / c causing the
>> effect that c is known non-zero after the statement if there were no
>> traps, or https://gcc.gnu.org/bugzilla/show_bug.cgi?id=31178 , which
>> after 15 years, we can now simply indicate that for  a = b >> c , its
>> only defined behaviour if c is in the range [0, PRECISION(b)]
>>
>> So its basically just a generalization of how we track known values
>> within the range system.
> Sure.  Just the name is harmful :-(
>
Still waiting for a suggestion, since "side effect" is the description 
that made sense to me :-)

Andrew


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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-20  2:14           ` Andrew MacLeod
@ 2022-05-20  6:25             ` Richard Biener
  2022-05-20  6:38               ` Alexander Monakov
  2022-05-20  8:13               ` [PATCH] Add divide by zero side effect Eric Botcazou
  0 siblings, 2 replies; 26+ messages in thread
From: Richard Biener @ 2022-05-20  6:25 UTC (permalink / raw)
  To: Andrew MacLeod; +Cc: Segher Boessenkool, gcc-patches

On Fri, May 20, 2022 at 4:15 AM Andrew MacLeod via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On 5/19/22 18:23, Segher Boessenkool wrote:
> > On Thu, May 19, 2022 at 09:22:32AM -0400, Andrew MacLeod wrote:
> >> On 5/18/22 16:40, Segher Boessenkool wrote:
> >>> On Wed, May 18, 2022 at 04:24:06PM -0400, Andrew MacLeod wrote:
> >>>> On 5/18/22 14:13, Segher Boessenkool wrote:
> >>>>> "Side effect" already has a meaning, very commonly used in language
> >>>>> theory, and even in the C standard itself: a function has a side effect
> >>>>> if it does something more than just return a value: if it changes state.
> >>>>> This can be some I/O, or it can just be writing to some non-local data.
> >>>>>
> >>>>> Side effects are crucial to what a compiler does, and they are used all
> >>>>> over the place (the gcc/ dir has some thousand mentions of it for
> >>>>> example).
> >>>>>
> >>>>> Please don't make life hard for everyone by overloading this term.
> >>>>>
> >>>> I'm open to suggestions for a better term!
> >>> Glad to hear that, and this isn't set in stione yet!
> >>>
> >>>> Is there a commonly used alternate term to describe an observable effect
> >>>> on the value of an input operand?
> >>> I'd use something with "known" in the name.  But:
> >>>
> >>> As far as I understand what you are doing this is not an effect on the
> >>> operand at all!  It cannot be one even, the operand is an input only
> >>> after all.  Instead, it changes what is known about the value of that
> >>> input: it cannot be 0 in this case, it is known to not be 0.
> >>>
> >>> This is similar to other known value things we have in GCC already.  Can
> >>> you not just use one of those, even?  What are the benefit to this new
> >>> abstraction?
> >> Well, This is a component of ranger tracking value ranges..  it is
> >> recording the "side-effect" of the stmt on the known range of an object.
> >> The file is called  "gimple-range-side-effect.h"
> > So the file name is confusingly wrong as well.
> >
> >> Its a generalization of how ranger tracks non-null pointer values,
> >> enabling it to track arbitrary observable ranges values. (The old
> >> mechanism also utilized immediate-use chains, which prevented it from
> >> being utilized in gimple-folding)
> >>
> >> WIth this change, we can also track things like a = b / c causing the
> >> effect that c is known non-zero after the statement if there were no
> >> traps, or https://gcc.gnu.org/bugzilla/show_bug.cgi?id=31178 , which
> >> after 15 years, we can now simply indicate that for  a = b >> c , its
> >> only defined behaviour if c is in the range [0, PRECISION(b)]
> >>
> >> So its basically just a generalization of how we track known values
> >> within the range system.
> > Sure.  Just the name is harmful :-(
> >
> Still waiting for a suggestion, since "side effect" is the description
> that made sense to me :-)

I think side-effect captures it quite well even if it overlaps with a term
used in language standards.  Doing c = a << b has the side-effect on
imposing a range on 'b' rather than just affecting 'c' (and its range).
You could call it 'alternate effect' but that sounds just awkward ;)

Richard.

> Andrew
>

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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-20  6:25             ` Richard Biener
@ 2022-05-20  6:38               ` Alexander Monakov
  2022-05-20  8:04                 ` Richard Biener
  2022-05-20  8:11                 ` Eric Botcazou
  2022-05-20  8:13               ` [PATCH] Add divide by zero side effect Eric Botcazou
  1 sibling, 2 replies; 26+ messages in thread
From: Alexander Monakov @ 2022-05-20  6:38 UTC (permalink / raw)
  To: Richard Biener; +Cc: Andrew MacLeod, gcc-patches, Segher Boessenkool

On Fri, 20 May 2022, Richard Biener via Gcc-patches wrote:

> > Still waiting for a suggestion, since "side effect" is the description
> > that made sense to me :-)
> 
> I think side-effect captures it quite well even if it overlaps with a term
> used in language standards.  Doing c = a << b has the side-effect on
> imposing a range on 'b' rather than just affecting 'c' (and its range).
> You could call it 'alternate effect' but that sounds just awkward ;)

I suggest 'deduce', 'deduction', 'deducing a range'. What the code is actually
doing is deducing that 'b' in 'a / b' cannot be zero. Function in GCC might be
called like 'deduce_ranges_from_stmt'.

Please don't overload 'side effect' if possible.

Alexander

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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-20  6:38               ` Alexander Monakov
@ 2022-05-20  8:04                 ` Richard Biener
  2022-05-20  8:17                   ` Alexander Monakov
  2022-05-20  8:11                 ` Eric Botcazou
  1 sibling, 1 reply; 26+ messages in thread
From: Richard Biener @ 2022-05-20  8:04 UTC (permalink / raw)
  To: Alexander Monakov; +Cc: Andrew MacLeod, gcc-patches, Segher Boessenkool

On Fri, May 20, 2022 at 8:38 AM Alexander Monakov <amonakov@ispras.ru> wrote:
>
> On Fri, 20 May 2022, Richard Biener via Gcc-patches wrote:
>
> > > Still waiting for a suggestion, since "side effect" is the description
> > > that made sense to me :-)
> >
> > I think side-effect captures it quite well even if it overlaps with a term
> > used in language standards.  Doing c = a << b has the side-effect on
> > imposing a range on 'b' rather than just affecting 'c' (and its range).
> > You could call it 'alternate effect' but that sounds just awkward ;)
>
> I suggest 'deduce', 'deduction', 'deducing a range'. What the code is actually
> doing is deducing that 'b' in 'a / b' cannot be zero. Function in GCC might be
> called like 'deduce_ranges_from_stmt'.

So how would you call determining the range of 'c' from the ranges of
'a' and 'b',
isn't that 'deduction' as well?

> Please don't overload 'side effect' if possible.
>
> Alexander

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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-20  6:38               ` Alexander Monakov
  2022-05-20  8:04                 ` Richard Biener
@ 2022-05-20  8:11                 ` Eric Botcazou
  2022-05-20 14:39                   ` Segher Boessenkool
  1 sibling, 1 reply; 26+ messages in thread
From: Eric Botcazou @ 2022-05-20  8:11 UTC (permalink / raw)
  To: Alexander Monakov; +Cc: Richard Biener, gcc-patches, Segher Boessenkool

> I suggest 'deduce', 'deduction', 'deducing a range'. What the code is
> actually doing is deducing that 'b' in 'a / b' cannot be zero. Function in
> GCC might be called like 'deduce_ranges_from_stmt'.

Or "infer", "inference", "inferring a range".

> Please don't overload 'side effect' if possible.

Agreed, "side effect" is something precisely defined in the compiler context.

-- 
Eric Botcazou



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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-20  6:25             ` Richard Biener
  2022-05-20  6:38               ` Alexander Monakov
@ 2022-05-20  8:13               ` Eric Botcazou
  1 sibling, 0 replies; 26+ messages in thread
From: Eric Botcazou @ 2022-05-20  8:13 UTC (permalink / raw)
  To: Richard Biener; +Cc: Andrew MacLeod, gcc-patches, Segher Boessenkool

> I think side-effect captures it quite well even if it overlaps with a term
> used in language standards.

IMO it's very confusing, see the subject: "Add divide by zero side effect".
The only side effect of dividing by zero is (possibly) raising a trap.

-- 
Eric Botcazou



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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-20  8:04                 ` Richard Biener
@ 2022-05-20  8:17                   ` Alexander Monakov
  2022-05-20 11:49                     ` Richard Biener
  0 siblings, 1 reply; 26+ messages in thread
From: Alexander Monakov @ 2022-05-20  8:17 UTC (permalink / raw)
  To: Richard Biener; +Cc: Andrew MacLeod, gcc-patches, Segher Boessenkool

On Fri, 20 May 2022, Richard Biener wrote:

> On Fri, May 20, 2022 at 8:38 AM Alexander Monakov <amonakov@ispras.ru> wrote:
> >
> > On Fri, 20 May 2022, Richard Biener via Gcc-patches wrote:
> >
> > > > Still waiting for a suggestion, since "side effect" is the description
> > > > that made sense to me :-)
> > >
> > > I think side-effect captures it quite well even if it overlaps with a term
> > > used in language standards.  Doing c = a << b has the side-effect on
> > > imposing a range on 'b' rather than just affecting 'c' (and its range).
> > > You could call it 'alternate effect' but that sounds just awkward ;)
> >
> > I suggest 'deduce', 'deduction', 'deducing a range'. What the code is actually
> > doing is deducing that 'b' in 'a / b' cannot be zero. Function in GCC might be
> > called like 'deduce_ranges_from_stmt'.
> 
> So how would you call determining the range of 'c' from the ranges of
> 'a' and 'b', isn't that 'deduction' as well?

Kind of, yes, but for this sort of forward inference I imagine you're already
using 'propagate [ranges through a stmt]', like in 'value range propagation'.

If you'd like to avoid 'propagate'/'deduce' asymmetry, I could suggest
'forward inference' / 'backward inference', but I like it a bit less.

Alexander

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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-20  8:17                   ` Alexander Monakov
@ 2022-05-20 11:49                     ` Richard Biener
  2022-05-22 18:55                       ` Alexander Monakov
  0 siblings, 1 reply; 26+ messages in thread
From: Richard Biener @ 2022-05-20 11:49 UTC (permalink / raw)
  To: Alexander Monakov; +Cc: Andrew MacLeod, gcc-patches, Segher Boessenkool

On Fri, May 20, 2022 at 10:17 AM Alexander Monakov <amonakov@ispras.ru> wrote:
>
> On Fri, 20 May 2022, Richard Biener wrote:
>
> > On Fri, May 20, 2022 at 8:38 AM Alexander Monakov <amonakov@ispras.ru> wrote:
> > >
> > > On Fri, 20 May 2022, Richard Biener via Gcc-patches wrote:
> > >
> > > > > Still waiting for a suggestion, since "side effect" is the description
> > > > > that made sense to me :-)
> > > >
> > > > I think side-effect captures it quite well even if it overlaps with a term
> > > > used in language standards.  Doing c = a << b has the side-effect on
> > > > imposing a range on 'b' rather than just affecting 'c' (and its range).
> > > > You could call it 'alternate effect' but that sounds just awkward ;)
> > >
> > > I suggest 'deduce', 'deduction', 'deducing a range'. What the code is actually
> > > doing is deducing that 'b' in 'a / b' cannot be zero. Function in GCC might be
> > > called like 'deduce_ranges_from_stmt'.
> >
> > So how would you call determining the range of 'c' from the ranges of
> > 'a' and 'b', isn't that 'deduction' as well?
>
> Kind of, yes, but for this sort of forward inference I imagine you're already
> using 'propagate [ranges through a stmt]', like in 'value range propagation'.
>
> If you'd like to avoid 'propagate'/'deduce' asymmetry, I could suggest
> 'forward inference' / 'backward inference', but I like it a bit less.

Hmm, maybe "guarantees" - if the stmt executed (without traps) then
it's guaranteed that the divisor isn't zero.  I've almost said 'assertions'
but then asserts also have separate meanings, not to mention ASSERT_EXPR
as currently used by the old-style VRP.

Richard.

>
> Alexander

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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-18 20:24   ` Andrew MacLeod
  2022-05-18 20:40     ` Segher Boessenkool
@ 2022-05-20 12:09     ` Alexandre Oliva
  1 sibling, 0 replies; 26+ messages in thread
From: Alexandre Oliva @ 2022-05-20 12:09 UTC (permalink / raw)
  To: Andrew MacLeod via Gcc-patches; +Cc: Segher Boessenkool, Andrew MacLeod

On May 18, 2022, Andrew MacLeod via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:

> I'm open to suggestions for a better term!

How about inference?

-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>

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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-20  8:11                 ` Eric Botcazou
@ 2022-05-20 14:39                   ` Segher Boessenkool
  2022-05-20 19:18                     ` Bernhard Reutner-Fischer
  0 siblings, 1 reply; 26+ messages in thread
From: Segher Boessenkool @ 2022-05-20 14:39 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: Alexander Monakov, Richard Biener, gcc-patches

On Fri, May 20, 2022 at 10:11:32AM +0200, Eric Botcazou wrote:
> > I suggest 'deduce', 'deduction', 'deducing a range'. What the code is
> > actually doing is deducing that 'b' in 'a / b' cannot be zero. Function in
> > GCC might be called like 'deduce_ranges_from_stmt'.
> 
> Or "infer", "inference", "inferring a range".

"Infer" is great here, yes!

infer (verb):
  deduce or conclude (something) from evidence and reasoning rather than
  from explicit statements

It has exactly the connotation wanted here, I would say.


Segher

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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-20 14:39                   ` Segher Boessenkool
@ 2022-05-20 19:18                     ` Bernhard Reutner-Fischer
  2022-05-25 14:35                       ` [COMMITTED] Use infer instead of side-effect for ranges Andrew MacLeod
  0 siblings, 1 reply; 26+ messages in thread
From: Bernhard Reutner-Fischer @ 2022-05-20 19:18 UTC (permalink / raw)
  To: gcc-patches, Segher Boessenkool, Eric Botcazou
  Cc: Alexander Monakov, gcc-patches

On 20 May 2022 16:39:20 CEST, Segher Boessenkool <segher@kernel.crashing.org> wrote:
>On Fri, May 20, 2022 at 10:11:32AM +0200, Eric Botcazou wrote:
>> > I suggest 'deduce', 'deduction', 'deducing a range'. What the code is
>> > actually doing is deducing that 'b' in 'a / b' cannot be zero. Function in
>> > GCC might be called like 'deduce_ranges_from_stmt'.
>> 
>> Or "infer", "inference", "inferring a range".
>
>"Infer" is great here, yes!
>
>infer (verb):
>  deduce or conclude (something) from evidence and reasoning rather than
>  from explicit statements
>
>It has exactly the connotation wanted here, I would say.

Infer, deduct, refine
Sound all plausible, a native speaker should probably help decide the bikeshed :-)
thanks,

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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-20 11:49                     ` Richard Biener
@ 2022-05-22 18:55                       ` Alexander Monakov
  0 siblings, 0 replies; 26+ messages in thread
From: Alexander Monakov @ 2022-05-22 18:55 UTC (permalink / raw)
  To: Richard Biener; +Cc: Andrew MacLeod, gcc-patches, Segher Boessenkool

On Fri, 20 May 2022, Richard Biener wrote:

> > > > I suggest 'deduce', 'deduction', 'deducing a range'. What the code is actually
> > > > doing is deducing that 'b' in 'a / b' cannot be zero. Function in GCC might be
> > > > called like 'deduce_ranges_from_stmt'.
> > >
> > > So how would you call determining the range of 'c' from the ranges of
> > > 'a' and 'b', isn't that 'deduction' as well?
> >
> > Kind of, yes, but for this sort of forward inference I imagine you're already
> > using 'propagate [ranges through a stmt]', like in 'value range propagation'.
> >
> > If you'd like to avoid 'propagate'/'deduce' asymmetry, I could suggest
> > 'forward inference' / 'backward inference', but I like it a bit less.
> 
> Hmm, maybe "guarantees" - if the stmt executed (without traps) then
> it's guaranteed that the divisor isn't zero.  I've almost said 'assertions'
> but then asserts also have separate meanings, not to mention ASSERT_EXPR
> as currently used by the old-style VRP.

I feel the word 'assumptions' captures that nicely.

Alexander

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

* [COMMITTED] Use infer instead of side-effect for ranges.
  2022-05-20 19:18                     ` Bernhard Reutner-Fischer
@ 2022-05-25 14:35                       ` Andrew MacLeod
  2022-05-25 16:29                         ` Segher Boessenkool
  0 siblings, 1 reply; 26+ messages in thread
From: Andrew MacLeod @ 2022-05-25 14:35 UTC (permalink / raw)
  To: Bernhard Reutner-Fischer, gcc-patches, Segher Boessenkool, Eric Botcazou
  Cc: Alexander Monakov

[-- Attachment #1: Type: text/plain, Size: 1227 bytes --]

On 5/20/22 15:18, Bernhard Reutner-Fischer via Gcc-patches wrote:
> On 20 May 2022 16:39:20 CEST, Segher Boessenkool <segher@kernel.crashing.org> wrote:
>> On Fri, May 20, 2022 at 10:11:32AM +0200, Eric Botcazou wrote:
>>>> I suggest 'deduce', 'deduction', 'deducing a range'. What the code is
>>>> actually doing is deducing that 'b' in 'a / b' cannot be zero. Function in
>>>> GCC might be called like 'deduce_ranges_from_stmt'.
>>> Or "infer", "inference", "inferring a range".
>> "Infer" is great here, yes!
>>
>> infer (verb):
>>   deduce or conclude (something) from evidence and reasoning rather than
>>   from explicit statements
>>
>> It has exactly the connotation wanted here, I would say.
> Infer, deduct, refine
> Sound all plausible, a native speaker should probably help decide the bikeshed :-)
> thanks,
>
Executive decision made.

gimple-range-side-effect.{h,cc}   -> gimple-range-infer.{h,cc}

class stmt_side_effects  -->  class gimple_infer_range

class side_effect_manager  -->  class infer_range_manager

various APIs/comments with the term "side effect"  renamed to "infer 
range" or some variation of tense.

Bootstraps from scratch on x86_64-pc-linux-gnu with no regressions.  pushed.

Andrew


[-- Attachment #2: rename.diff --]
[-- Type: text/x-patch, Size: 19108 bytes --]

commit 156d7d8dbc8d65d3958486bc4112a7279935e47d
Author: Andrew MacLeod <amacleod@redhat.com>
Date:   Tue May 24 11:32:42 2022 -0400

    Use infer instead of side-effect for ranges.
    
    Rename the files and classes to reflect the term infer rather than side-effect.
    
            * Makefile.in (OBJS): Use gimple-range-infer.o.
            * gimple-range-cache.cc (ranger_cache::fill_block_cache): Change msg.
            (ranger_cache::range_from_dom): Rename var side_effect to infer.
            (ranger_cache::apply_inferred_ranges): Rename from apply_side_effects.
            * gimple-range-cache.h: Include gimple-range-infer.h.
            (class ranger_cache): Adjust prototypes, use infer_range_manager.
            * gimple-range-infer.cc: Rename from gimple-range-side-effects.cc.
            (gimple_infer_range::*): Rename from stmt_side_effects.
            (infer_range_manager::*): Rename from side_effect_manager.
            * gimple-range-side-effect.cc: Rename.
            * gimple-range-side-effect.h: Rename.
            * gimple-range-infer.h: Rename from gimple-range-side-effects.h.
            (class gimple_infer_range): Rename from stmt_side_effects.
            (class infer_range_manager): Rename from side_effect_manager.
            * gimple-range.cc (gimple_ranger::register_inferred_ranges): Rename
            from register_side_effects.
            * gimple-range.h (register_inferred_ranges): Adjust prototype.
            * range-op.h: Adjust comment.
            * tree-vrp.cc (rvrp_folder::pre_fold_bb): Use register_inferred_ranges.
            (rvrp_folder::post_fold_bb): Use register_inferred_ranges.

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 97e5450ecb5..731d8dd2a69 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1410,7 +1410,7 @@ OBJS = \
 	gimple-range-edge.o \
 	gimple-range-fold.o \
 	gimple-range-gori.o \
-	gimple-range-side-effect.o \
+	gimple-range-infer.o \
 	gimple-range-trace.o \
 	gimple-ssa-backprop.o \
 	gimple-ssa-evrp.o \
diff --git a/gcc/gimple-range-cache.cc b/gcc/gimple-range-cache.cc
index c726393b380..5d5e2bfd0c3 100644
--- a/gcc/gimple-range-cache.cc
+++ b/gcc/gimple-range-cache.cc
@@ -944,7 +944,7 @@ bool
 ranger_cache::edge_range (irange &r, edge e, tree name, enum rfd_mode mode)
 {
   exit_range (r, name, e->src, mode);
-  // If this is not an abnormal edge, check for side effects on exit.
+  // If this is not an abnormal edge, check for inferred ranges on exit.
   if ((e->flags & (EDGE_EH | EDGE_ABNORMAL)) == 0)
     m_exit.maybe_adjust_range (r, name, e->src);
   int_range_max er;
@@ -1251,12 +1251,12 @@ ranger_cache::fill_block_cache (tree name, basic_block bb, basic_block def_bb)
 	    }
 
 	  // Regardless of whether we have visited pred or not, if the
-	  // pred has side_effects, revisit this block.
+	  // pred has inferred ranges, revisit this block.
 	  // Don't search the DOM tree.
 	  if (m_exit.has_range_p (name, pred))
 	    {
 	      if (DEBUG_RANGE_CACHE)
-		fprintf (dump_file, "side effect: update ");
+		fprintf (dump_file, "Inferred range: update ");
 	      m_update->add (node);
 	    }
 
@@ -1317,8 +1317,8 @@ ranger_cache::range_from_dom (irange &r, tree name, basic_block start_bb,
   basic_block bb;
   basic_block prev_bb = start_bb;
 
-  // Track any side effects seen
-  int_range_max side_effect (TREE_TYPE (name));
+  // Track any inferred ranges seen.
+  int_range_max infer (TREE_TYPE (name));
 
   // Range on entry to the DEF block should not be queried.
   gcc_checking_assert (start_bb != def_bb);
@@ -1332,8 +1332,8 @@ ranger_cache::range_from_dom (irange &r, tree name, basic_block start_bb,
        bb;
        prev_bb = bb, bb = get_immediate_dominator (CDI_DOMINATORS, bb))
     {
-      // Accumulate any block exit side effects.
-      m_exit.maybe_adjust_range (side_effect, name, bb);
+      // Accumulate any block exit inferred ranges.
+      m_exit.maybe_adjust_range (infer, name, bb);
 
       // This block has an outgoing range.
       if (m_gori.has_edge_range_p (name, bb))
@@ -1399,7 +1399,7 @@ ranger_cache::range_from_dom (irange &r, tree name, basic_block start_bb,
       if (m_gori.outgoing_edge_range_p (er, e, name, *this))
 	{
 	  r.intersect (er);
-	  // If this is a normal edge, apply any side effects.
+	  // If this is a normal edge, apply any inferred ranges.
 	  if ((e->flags & (EDGE_EH | EDGE_ABNORMAL)) == 0)
 	    m_exit.maybe_adjust_range (r, name, bb);
 
@@ -1415,7 +1415,7 @@ ranger_cache::range_from_dom (irange &r, tree name, basic_block start_bb,
 
   // Apply non-null if appropriate.
   if (!has_abnormal_call_or_eh_pred_edge_p (start_bb))
-    r.intersect (side_effect);
+    r.intersect (infer);
 
   if (DEBUG_RANGE_CACHE)
     {
@@ -1430,14 +1430,14 @@ ranger_cache::range_from_dom (irange &r, tree name, basic_block start_bb,
 // any operands on stmt S to nonnull.
 
 void
-ranger_cache::apply_side_effects (gimple *s)
+ranger_cache::apply_inferred_ranges (gimple *s)
 {
   int_range_max r;
   bool update = true;
 
   basic_block bb = gimple_bb (s);
-  stmt_side_effects se(s);
-  if (se.num () == 0)
+  gimple_infer_range infer(s);
+  if (infer.num () == 0)
     return;
 
   // Do not update the on-netry cache for block ending stmts.
@@ -1452,15 +1452,15 @@ ranger_cache::apply_side_effects (gimple *s)
 	update = false;
     }
 
-  for (unsigned x = 0; x < se.num (); x++)
+  for (unsigned x = 0; x < infer.num (); x++)
     {
-      tree name = se.name (x);
-      m_exit.add_range (name, bb, se.range (x));
+      tree name = infer.name (x);
+      m_exit.add_range (name, bb, infer.range (x));
       if (update)
 	{
 	  if (!m_on_entry.get_bb_range (r, name, bb))
 	    exit_range (r, name, bb, RFD_READ_ONLY);
-	  if (r.intersect (se.range (x)))
+	  if (r.intersect (infer.range (x)))
 	    m_on_entry.set_bb_range (name, bb, r);
 	}
     }
diff --git a/gcc/gimple-range-cache.h b/gcc/gimple-range-cache.h
index 555fe32513f..d56e56c201c 100644
--- a/gcc/gimple-range-cache.h
+++ b/gcc/gimple-range-cache.h
@@ -22,7 +22,7 @@ along with GCC; see the file COPYING3.  If not see
 #define GCC_SSA_RANGE_CACHE_H
 
 #include "gimple-range-gori.h" 
-#include "gimple-range-side-effect.h"
+#include "gimple-range-infer.h"
 
 // This class manages a vector of pointers to ssa_block ranges.  It
 // provides the basis for the "range on entry" cache for all
@@ -87,9 +87,9 @@ public:
 
   void propagate_updated_value (tree name, basic_block bb);
 
-  void apply_side_effects (gimple *s);
+  void apply_inferred_ranges (gimple *s);
   gori_compute m_gori;
-  side_effect_manager m_exit;
+  infer_range_manager m_exit;
 
   void dump_bb (FILE *f, basic_block bb);
   virtual void dump (FILE *f) override;
diff --git a/gcc/gimple-range-side-effect.cc b/gcc/gimple-range-infer.cc
similarity index 79%
rename from gcc/gimple-range-side-effect.cc
rename to gcc/gimple-range-infer.cc
index 2c8c77dc569..8e25830849f 100644
--- a/gcc/gimple-range-side-effect.cc
+++ b/gcc/gimple-range-infer.cc
@@ -1,4 +1,4 @@
-/* Gimple range side effect implementation.
+/* Gimple range inference implementation.
    Copyright (C) 2022 Free Software Foundation, Inc.
    Contributed by Andrew MacLeod <amacleod@redhat.com>.
 
@@ -37,7 +37,7 @@ along with GCC; see the file COPYING3.  If not see
 
 // Adapted from infer_nonnull_range_by_dereference and check_loadstore
 // to process nonnull ssa_name OP in S.  DATA contains a pointer to a
-// stmt side effects instance.
+// stmt range inference instance.
 
 static bool
 non_null_loadstore (gimple *, tree op, tree, void *data)
@@ -49,16 +49,16 @@ non_null_loadstore (gimple *, tree op, tree, void *data)
       if (!targetm.addr_space.zero_address_valid (as))
 	{
 	  tree ssa = TREE_OPERAND (op, 0);
-	  ((stmt_side_effects *)data)->add_nonzero (ssa);
+	  ((gimple_infer_range *)data)->add_nonzero (ssa);
 	}
     }
   return false;
 }
 
-// Add NAME and RANGE to the the side effect summary.
+// Add NAME and RANGE to the the range inference summary.
 
 void
-stmt_side_effects::add_range (tree name, irange &range)
+gimple_infer_range::add_range (tree name, irange &range)
 {
   m_names[num_args] = name;
   m_ranges[num_args] = range;
@@ -66,10 +66,10 @@ stmt_side_effects::add_range (tree name, irange &range)
     num_args++;
 }
 
-// Add a nonzero range for NAME to the side effect summary.
+// Add a nonzero range for NAME to the range inference summary.
 
 void
-stmt_side_effects::add_nonzero (tree name)
+gimple_infer_range::add_nonzero (tree name)
 {
   if (!gimple_range_ssa_p (name))
     return;
@@ -78,10 +78,10 @@ stmt_side_effects::add_nonzero (tree name)
   add_range (name, nz);
 }
 
-// Process S for side effects and fill in the summary list.
-// This is the routine where new side effects should be added.
+// Process S for range inference and fill in the summary list.
+// This is the routine where new inferred ranges should be added.
 
-stmt_side_effects::stmt_side_effects (gimple *s)
+gimple_infer_range::gimple_infer_range (gimple *s)
 {
   num_args = 0;
 
@@ -120,7 +120,7 @@ stmt_side_effects::stmt_side_effects (gimple *s)
 
 // -------------------------------------------------------------------------
 
-// This class is an element in list of side effect ranges.
+// This class is an element in list of infered ranges.
 
 class exit_range
 {
@@ -134,7 +134,7 @@ public:
 // Otherwise return NULL.
 
 exit_range *
-side_effect_manager::exit_range_head::find_ptr (tree ssa)
+infer_range_manager::exit_range_head::find_ptr (tree ssa)
 {
   // Return NULL if SSA is not in this list.
   if (!m_names || !bitmap_bit_p (m_names, SSA_NAME_VERSION (ssa)))
@@ -147,11 +147,11 @@ side_effect_manager::exit_range_head::find_ptr (tree ssa)
   return NULL;
 }
 
-// Construct a side effects manager.  DO_SEARCH indicates whether an immediate
+// Construct a range infer manager.  DO_SEARCH indicates whether an immediate
 // use scan should be made the first time a name is processed.  This is for
 // on-demand clients who may not visit every statement and may miss uses.
 
-side_effect_manager::side_effect_manager (bool do_search)
+infer_range_manager::infer_range_manager (bool do_search)
 {
   bitmap_obstack_initialize (&m_bitmaps);
   m_on_exit.create (0);
@@ -168,9 +168,9 @@ side_effect_manager::side_effect_manager (bool do_search)
   m_nonzero.safe_grow_cleared (num_ssa_names + 1);
 }
 
-// Destruct a side effects manager.
+// Destruct a range infer manager.
 
-side_effect_manager::~side_effect_manager ()
+infer_range_manager::~infer_range_manager ()
 {
   m_nonzero.release ();
   obstack_free (&m_list_obstack, NULL);
@@ -182,7 +182,7 @@ side_effect_manager::~side_effect_manager ()
 // the cache, creating it if necessary.
 
 const irange&
-side_effect_manager::get_nonzero (tree name)
+infer_range_manager::get_nonzero (tree name)
 {
   unsigned v = SSA_NAME_VERSION (name);
   if (v >= m_nonzero.length ())
@@ -195,10 +195,10 @@ side_effect_manager::get_nonzero (tree name)
   return *(m_nonzero[v]);
 }
 
-// Return TRUE if NAME has a side effect range in block BB.
+// Return TRUE if NAME has a range inference in block BB.
 
 bool
-side_effect_manager::has_range_p (tree name, basic_block bb)
+infer_range_manager::has_range_p (tree name, basic_block bb)
 {
   // Check if this is an immediate use search model.
   if (m_seen && !bitmap_bit_p (m_seen, SSA_NAME_VERSION (name)))
@@ -213,11 +213,11 @@ side_effect_manager::has_range_p (tree name, basic_block bb)
   return true;
 }
 
-// Return TRUE if NAME has a side effect range in block BB, and adjust range R
+// Return TRUE if NAME has a range inference in block BB, and adjust range R
 // to include it.
 
 bool
-side_effect_manager::maybe_adjust_range (irange &r, tree name, basic_block bb)
+infer_range_manager::maybe_adjust_range (irange &r, tree name, basic_block bb)
 {
   if (!has_range_p (name, bb))
     return false;
@@ -227,10 +227,10 @@ side_effect_manager::maybe_adjust_range (irange &r, tree name, basic_block bb)
   return r.intersect (*(ptr->range));
 }
 
-// Add range R as a side effect for NAME in block BB.
+// Add range R as an inferred range for NAME in block BB.
 
 void
-side_effect_manager::add_range (tree name, basic_block bb, const irange &r)
+infer_range_manager::add_range (tree name, basic_block bb, const irange &r)
 {
   if (bb->index >= (int)m_on_exit.length ())
     m_on_exit.safe_grow_cleared (last_basic_block_for_fn (cfun) + 1);
@@ -272,18 +272,18 @@ side_effect_manager::add_range (tree name, basic_block bb, const irange &r)
   m_on_exit[bb->index].head = ptr;
 }
 
-// Add a non-zero side effect for NAME in block BB.
+// Add a non-zero inferred range for NAME in block BB.
 
 void
-side_effect_manager::add_nonzero (tree name, basic_block bb)
+infer_range_manager::add_nonzero (tree name, basic_block bb)
 {
   add_range (name, bb, get_nonzero (name));
 }
 
-// Follow immediate use chains and find all side effects for NAME.
+// Follow immediate use chains and find all inferred ranges for NAME.
 
 void
-side_effect_manager::register_all_uses (tree name)
+infer_range_manager::register_all_uses (tree name)
 {
   gcc_checking_assert (m_seen);
 
@@ -296,15 +296,15 @@ side_effect_manager::register_all_uses (tree name)
   use_operand_p use_p;
   imm_use_iterator iter;
 
-  // Loop over each immediate use and see if it has a side effect.
+  // Loop over each immediate use and see if it has an inferred range.
   FOR_EACH_IMM_USE_FAST (use_p, iter, name)
     {
       gimple *s = USE_STMT (use_p);
-      stmt_side_effects se (s);
-      for (unsigned x = 0; x < se.num (); x++)
+      gimple_infer_range infer (s);
+      for (unsigned x = 0; x < infer.num (); x++)
 	{
-	  if (name == se.name (x))
-	    add_range (name, gimple_bb (s), se.range (x));
+	  if (name == infer.name (x))
+	    add_range (name, gimple_bb (s), infer.range (x));
 	}
     }
 }
diff --git a/gcc/gimple-range-side-effect.h b/gcc/gimple-range-infer.h
similarity index 76%
rename from gcc/gimple-range-side-effect.h
rename to gcc/gimple-range-infer.h
index 848d94ba6d7..629436753e9 100644
--- a/gcc/gimple-range-side-effect.h
+++ b/gcc/gimple-range-infer.h
@@ -1,4 +1,4 @@
-/* Header file for gimple range side effects.
+/* Header file for gimple range inference.
    Copyright (C) 2022 Free Software Foundation, Inc.
    Contributed by Andrew MacLeod <amacleod@redhat.com>.
 
@@ -21,15 +21,17 @@ along with GCC; see the file COPYING3.  If not see
 #ifndef GCC_GIMPLE_RANGE_SIDE_H
 #define GCC_GIMPLE_RANGE_SIDE_H
 
-// This class manages an on-demand summary of side effects for a statement.
-// It can be instantiated as required and provides a list of side effects.
+// Inferred ranges are ranges which are applied to use operands as a by product
+// of executing an operation.
 
-// New side effects should added in the constructor of this class.
+// This class manages an on-demand summary of inferred ranges for a statement.
+// It can be instantiated as required and provides a list of inferred ranges.
+// New inferred ranges should added in the constructor of this class.
 
-class stmt_side_effects
+class gimple_infer_range
 {
 public:
-  stmt_side_effects (gimple *s);
+  gimple_infer_range (gimple *s);
   inline unsigned num () const { return num_args; }
   inline tree name (unsigned index) const
     { gcc_checking_assert (index < num_args); return m_names[index]; }
@@ -45,17 +47,17 @@ private:
   inline void bump_index () { if (num_args < size_limit - 1) num_args++; }
 };
 
-// This class manages a list of side effect ranges for each basic block.
-// As side effects are seen, they can be registered to a block and later
+// This class manages a list of inferred ranges for each basic block.
+// As inferences are made, they can be registered to a block and later
 // queried.  WHen constructed with a TRUE flag, immediate uses chains are
 // followed the first time a name is referenced and block populated if
-// thre are any side effects.
+// there are any inferred ranges.
 
-class side_effect_manager
+class infer_range_manager
 {
 public:
-  side_effect_manager (bool do_search);
-  ~side_effect_manager ();
+  infer_range_manager (bool do_search);
+  ~infer_range_manager ();
   void add_range (tree name, basic_block bb, const irange &r);
   void add_nonzero (tree name, basic_block bb);
   bool has_range_p (tree name, basic_block bb);
diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index f5e9e77bc71..08a9c01e91a 100644
--- a/gcc/gimple-range.cc
+++ b/gcc/gimple-range.cc
@@ -446,12 +446,11 @@ gimple_ranger::fold_stmt (gimple_stmt_iterator *gsi, tree (*valueize) (tree))
   return ret;
 }
 
-// Called during dominator walks to register any side effects that take effect
-// from this point forward.  Current release is only for tracking non-null
-// within a block.
+// Called during dominator walks to register any inferred ranges that take
+// effect from this point forward.  
 
 void
-gimple_ranger::register_side_effects (gimple *s)
+gimple_ranger::register_inferred_ranges (gimple *s)
 {
   // First, export the LHS if it is a new global range.
   tree lhs = gimple_get_lhs (s);
@@ -475,7 +474,7 @@ gimple_ranger::register_side_effects (gimple *s)
 	  fputc ('\n', dump_file);
 	}
     }
-  m_cache.apply_side_effects (s);
+  m_cache.apply_inferred_ranges (s);
 }
 
 // This routine will export whatever global ranges are known to GCC
diff --git a/gcc/gimple-range.h b/gcc/gimple-range.h
index 13d4c77883e..c67280dc1d2 100644
--- a/gcc/gimple-range.h
+++ b/gcc/gimple-range.h
@@ -60,7 +60,7 @@ public:
   void dump_bb (FILE *f, basic_block bb);
   auto_edge_flag non_executable_edge_flag;
   bool fold_stmt (gimple_stmt_iterator *gsi, tree (*) (tree));
-  void register_side_effects (gimple *s);
+  void register_inferred_ranges (gimple *s);
 protected:
   bool fold_range_internal (irange &r, gimple *s, tree name);
   void prefill_name (irange &r, tree name);
diff --git a/gcc/range-op.h b/gcc/range-op.h
index 5fdda326d4b..300974fbb78 100644
--- a/gcc/range-op.h
+++ b/gcc/range-op.h
@@ -95,7 +95,7 @@ protected:
 		        const wide_int &lh_ub,
 		        const wide_int &rh_lb,
 		        const wide_int &rh_ub) const;
-  // Side effect of relation for generic fold_range clients.
+  // Effect of relation for generic fold_range clients.
   virtual bool op1_op2_relation_effect (irange &lhs_range, tree type,
 					const irange &op1_range,
 					const irange &op2_range,
diff --git a/gcc/tree-vrp.cc b/gcc/tree-vrp.cc
index 0784d658567..62ae5a967f3 100644
--- a/gcc/tree-vrp.cc
+++ b/gcc/tree-vrp.cc
@@ -4304,7 +4304,7 @@ public:
     m_pta->enter (bb);
     for (gphi_iterator gsi = gsi_start_phis (bb); !gsi_end_p (gsi);
 	 gsi_next (&gsi))
-      m_ranger->register_side_effects (gsi.phi ());
+      m_ranger->register_inferred_ranges (gsi.phi ());
   }
 
   void post_fold_bb (basic_block bb) override
@@ -4322,7 +4322,7 @@ public:
     bool ret = m_simplifier.simplify (gsi);
     if (!ret)
       ret = m_ranger->fold_stmt (gsi, follow_single_use_edges);
-    m_ranger->register_side_effects (gsi_stmt (*gsi));
+    m_ranger->register_inferred_ranges (gsi_stmt (*gsi));
     return ret;
   }
 

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

* Re: [COMMITTED] Use infer instead of side-effect for ranges.
  2022-05-25 14:35                       ` [COMMITTED] Use infer instead of side-effect for ranges Andrew MacLeod
@ 2022-05-25 16:29                         ` Segher Boessenkool
  0 siblings, 0 replies; 26+ messages in thread
From: Segher Boessenkool @ 2022-05-25 16:29 UTC (permalink / raw)
  To: Andrew MacLeod
  Cc: Bernhard Reutner-Fischer, gcc-patches, Eric Botcazou, Alexander Monakov

On Wed, May 25, 2022 at 10:35:31AM -0400, Andrew MacLeod wrote:
> Executive decision made.
> 
> gimple-range-side-effect.{h,cc}   -> gimple-range-infer.{h,cc}
> 
> class stmt_side_effects  -->  class gimple_infer_range
> 
> class side_effect_manager  -->  class infer_range_manager
> 
> various APIs/comments with the term "side effect"  renamed to "infer 
> range" or some variation of tense.

Thank you!


Segher

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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-17 18:39 [PATCH] Add divide by zero side effect Andrew MacLeod
  2022-05-18  6:28 ` Richard Biener
  2022-05-18 18:13 ` Segher Boessenkool
@ 2022-05-27 19:33 ` Andi Kleen
  2022-05-27 19:56   ` Andrew MacLeod
  2022-05-30 12:24   ` Richard Biener
  2 siblings, 2 replies; 26+ messages in thread
From: Andi Kleen @ 2022-05-27 19:33 UTC (permalink / raw)
  To: Andrew MacLeod via Gcc-patches

Andrew MacLeod via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>
> diff --git a/gcc/gimple-range-side-effect.cc b/gcc/gimple-range-side-effect.cc
> index 2c8c77dc569..548e4bea313 100644
> --- a/gcc/gimple-range-side-effect.cc
> +++ b/gcc/gimple-range-side-effect.cc
> @@ -116,6 +116,23 @@ stmt_side_effects::stmt_side_effects (gimple *s)
>      walk_stmt_load_store_ops (s, (void *)this, non_null_loadstore,
>  			      non_null_loadstore);
>  
> +  if (is_a<gassign *> (s))
> +    {
> +      switch (gimple_assign_rhs_code (s))
> +	{
> +	case TRUNC_DIV_EXPR:
> +	case CEIL_DIV_EXPR:
> +	case FLOOR_DIV_EXPR:
> +	case ROUND_DIV_EXPR:
> +	case EXACT_DIV_EXPR:
> +	  // Divide means operand 2 is not zero after this stmt.
> +	  if (gimple_range_ssa_p (gimple_assign_rhs2 (s)))
> +	    add_nonzero (gimple_assign_rhs2 (s));

Sorry I'm late, but how does this ensure the value is a integer?
I believe for floating point the assumption is not correct because
division by zero doesn't necessarily fault.

-Andi

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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-27 19:33 ` Andi Kleen
@ 2022-05-27 19:56   ` Andrew MacLeod
  2022-05-28 11:52     ` Eric Gallager
  2022-05-30 12:24   ` Richard Biener
  1 sibling, 1 reply; 26+ messages in thread
From: Andrew MacLeod @ 2022-05-27 19:56 UTC (permalink / raw)
  To: Andi Kleen, Andrew MacLeod via Gcc-patches

On 5/27/22 15:33, Andi Kleen wrote:
> Andrew MacLeod via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>> diff --git a/gcc/gimple-range-side-effect.cc b/gcc/gimple-range-side-effect.cc
>> index 2c8c77dc569..548e4bea313 100644
>> --- a/gcc/gimple-range-side-effect.cc
>> +++ b/gcc/gimple-range-side-effect.cc
>> @@ -116,6 +116,23 @@ stmt_side_effects::stmt_side_effects (gimple *s)
>>       walk_stmt_load_store_ops (s, (void *)this, non_null_loadstore,
>>   			      non_null_loadstore);
>>   
>> +  if (is_a<gassign *> (s))
>> +    {
>> +      switch (gimple_assign_rhs_code (s))
>> +	{
>> +	case TRUNC_DIV_EXPR:
>> +	case CEIL_DIV_EXPR:
>> +	case FLOOR_DIV_EXPR:
>> +	case ROUND_DIV_EXPR:
>> +	case EXACT_DIV_EXPR:
>> +	  // Divide means operand 2 is not zero after this stmt.
>> +	  if (gimple_range_ssa_p (gimple_assign_rhs2 (s)))
>> +	    add_nonzero (gimple_assign_rhs2 (s));
> Sorry I'm late, but how does this ensure the value is a integer?
> I believe for floating point the assumption is not correct because
> division by zero doesn't necessarily fault.
>
> -Andi
>
gimple_range_ssa_p() only returns non-zero when the value is an ssa_name 
whose type is supported by the range calculators... currently only 
integrals values.  When we support floating points we will have to add 
additional logic.

Andrew


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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-27 19:56   ` Andrew MacLeod
@ 2022-05-28 11:52     ` Eric Gallager
  0 siblings, 0 replies; 26+ messages in thread
From: Eric Gallager @ 2022-05-28 11:52 UTC (permalink / raw)
  To: Andrew MacLeod; +Cc: Andi Kleen, Andrew MacLeod via Gcc-patches

On Fri, May 27, 2022 at 3:57 PM Andrew MacLeod via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On 5/27/22 15:33, Andi Kleen wrote:
> > Andrew MacLeod via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> >> diff --git a/gcc/gimple-range-side-effect.cc b/gcc/gimple-range-side-effect.cc
> >> index 2c8c77dc569..548e4bea313 100644
> >> --- a/gcc/gimple-range-side-effect.cc
> >> +++ b/gcc/gimple-range-side-effect.cc
> >> @@ -116,6 +116,23 @@ stmt_side_effects::stmt_side_effects (gimple *s)
> >>       walk_stmt_load_store_ops (s, (void *)this, non_null_loadstore,
> >>                            non_null_loadstore);
> >>
> >> +  if (is_a<gassign *> (s))
> >> +    {
> >> +      switch (gimple_assign_rhs_code (s))
> >> +    {
> >> +    case TRUNC_DIV_EXPR:
> >> +    case CEIL_DIV_EXPR:
> >> +    case FLOOR_DIV_EXPR:
> >> +    case ROUND_DIV_EXPR:
> >> +    case EXACT_DIV_EXPR:
> >> +      // Divide means operand 2 is not zero after this stmt.
> >> +      if (gimple_range_ssa_p (gimple_assign_rhs2 (s)))
> >> +        add_nonzero (gimple_assign_rhs2 (s));
> > Sorry I'm late, but how does this ensure the value is a integer?
> > I believe for floating point the assumption is not correct because
> > division by zero doesn't necessarily fault.
> >
> > -Andi
> >
> gimple_range_ssa_p() only returns non-zero when the value is an ssa_name
> whose type is supported by the range calculators... currently only
> integrals values.  When we support floating points we will have to add
> additional logic.
>

Maybe add a comment to that effect, then? As a reminder...

> Andrew
>

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

* Re: [PATCH] Add divide by zero side effect.
  2022-05-27 19:33 ` Andi Kleen
  2022-05-27 19:56   ` Andrew MacLeod
@ 2022-05-30 12:24   ` Richard Biener
  1 sibling, 0 replies; 26+ messages in thread
From: Richard Biener @ 2022-05-30 12:24 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Andrew MacLeod via Gcc-patches

On Fri, May 27, 2022 at 9:34 PM Andi Kleen via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Andrew MacLeod via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> >
> > diff --git a/gcc/gimple-range-side-effect.cc b/gcc/gimple-range-side-effect.cc
> > index 2c8c77dc569..548e4bea313 100644
> > --- a/gcc/gimple-range-side-effect.cc
> > +++ b/gcc/gimple-range-side-effect.cc
> > @@ -116,6 +116,23 @@ stmt_side_effects::stmt_side_effects (gimple *s)
> >      walk_stmt_load_store_ops (s, (void *)this, non_null_loadstore,
> >                             non_null_loadstore);
> >
> > +  if (is_a<gassign *> (s))
> > +    {
> > +      switch (gimple_assign_rhs_code (s))
> > +     {
> > +     case TRUNC_DIV_EXPR:
> > +     case CEIL_DIV_EXPR:
> > +     case FLOOR_DIV_EXPR:
> > +     case ROUND_DIV_EXPR:
> > +     case EXACT_DIV_EXPR:
> > +       // Divide means operand 2 is not zero after this stmt.
> > +       if (gimple_range_ssa_p (gimple_assign_rhs2 (s)))
> > +         add_nonzero (gimple_assign_rhs2 (s));
>
> Sorry I'm late, but how does this ensure the value is a integer?
> I believe for floating point the assumption is not correct because
> division by zero doesn't necessarily fault.

non-integer divisions use a different TREE_CODE (RDIV_EXPR).

>
> -Andi

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

end of thread, other threads:[~2022-05-30 12:24 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-17 18:39 [PATCH] Add divide by zero side effect Andrew MacLeod
2022-05-18  6:28 ` Richard Biener
2022-05-18 13:20   ` Andrew MacLeod
2022-05-18 18:13 ` Segher Boessenkool
2022-05-18 20:24   ` Andrew MacLeod
2022-05-18 20:40     ` Segher Boessenkool
2022-05-19 13:22       ` Andrew MacLeod
2022-05-19 22:23         ` Segher Boessenkool
2022-05-20  2:14           ` Andrew MacLeod
2022-05-20  6:25             ` Richard Biener
2022-05-20  6:38               ` Alexander Monakov
2022-05-20  8:04                 ` Richard Biener
2022-05-20  8:17                   ` Alexander Monakov
2022-05-20 11:49                     ` Richard Biener
2022-05-22 18:55                       ` Alexander Monakov
2022-05-20  8:11                 ` Eric Botcazou
2022-05-20 14:39                   ` Segher Boessenkool
2022-05-20 19:18                     ` Bernhard Reutner-Fischer
2022-05-25 14:35                       ` [COMMITTED] Use infer instead of side-effect for ranges Andrew MacLeod
2022-05-25 16:29                         ` Segher Boessenkool
2022-05-20  8:13               ` [PATCH] Add divide by zero side effect Eric Botcazou
2022-05-20 12:09     ` Alexandre Oliva
2022-05-27 19:33 ` Andi Kleen
2022-05-27 19:56   ` Andrew MacLeod
2022-05-28 11:52     ` Eric Gallager
2022-05-30 12:24   ` Richard Biener

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 read-only IMAP folder(s) and NNTP newsgroup(s).