mbox series

[0/4] Replace nonzero_p with contains_zero_p

Message ID 20260805141722.3769280-2-aldy@quesejoda.com
Headers
Series Replace nonzero_p with contains_zero_p |

Message

Aldy Hernandez Aug. 5, 2026, 2:17 p.m. UTC 
  Per our discussion, nonzero_p() has always been a bit insane.  It only
returns true for ~[0,0] so even [13,13] is considered false.
Introduce contains_zero_p(), which from the looks of it, we've already
been hand crafting, as direct nonzero_p() calls were few.

How does this look?

Aldy


*** BLURB HERE ***

Aldy Hernandez (4):
  Add contains_zero_p to the vrange hierarchy.
  Use !contains_zero_p for known nonzero tests instead of nonzero_p.
  Implement the exact nonzero-set test without nonzero_p.
  Remove nonzero_p.

 gcc/analyzer/region-model.cc |  2 +-
 gcc/gimple-range-path.cc     |  2 +-
 gcc/range-op.cc              |  2 +-
 gcc/tree-ssa-structalias.cc  |  2 +-
 gcc/tree-ssanames.cc         |  2 +-
 gcc/tree-vrp.cc              |  2 +-
 gcc/value-range-storage.cc   | 21 +++++++++++++--
 gcc/value-range.cc           | 51 +++++++++++++-----------------------
 gcc/value-range.h            | 24 ++++++++++-------
 9 files changed, 57 insertions(+), 51 deletions(-)

Comments

Aldy Hernandez Aug. 5, 2026, 5:58 p.m. UTC | #1 
On Wed, Aug 05, 2026 at 04:17:19PM +0200, Aldy Hernandez wrote:
> Per our discussion, nonzero_p() has always been a bit insane.  It only
> returns true for ~[0,0] so even [13,13] is considered false.
> Introduce contains_zero_p(), which from the looks of it, we've already
> been hand crafting, as direct nonzero_p() calls were few.
> 
> How does this look?

BTW, I ran my usual LAPACK source files through each patch
independently, and there are no changes to assembly, so in theory this
whole set is surprisingly a non-functional change.

Aldy
Richard Biener Aug. 6, 2026, 8:28 a.m. UTC | #2 
On Wed, 5 Aug 2026, Aldy Hernandez wrote:

> On Wed, Aug 05, 2026 at 04:17:19PM +0200, Aldy Hernandez wrote:
> > Per our discussion, nonzero_p() has always been a bit insane.  It only
> > returns true for ~[0,0] so even [13,13] is considered false.
> > Introduce contains_zero_p(), which from the looks of it, we've already
> > been hand crafting, as direct nonzero_p() calls were few.
> > 
> > How does this look?
> 
> BTW, I ran my usual LAPACK source files through each patch
> independently, and there are no changes to assembly, so in theory this
> whole set is surprisingly a non-functional change.

You also add contains_zero_p for frange, but what does this actually
mean there?  irange already had contains_zero_p.

I suppose we can document contains_zero_p to mean that
for a value with the range val == T(0) may evaluate true?  So for
frange this means either -0.0 or 0.0?  irange implements
it in terms of contains_p which is already there for frange
and that implements it as >= && <=.

So I think it's all sound.

OK from my side.

Thanks,
Richard.
Aldy Hernandez Aug. 6, 2026, 10:46 a.m. UTC | #3 
On Thu, Aug 06, 2026 at 10:28:29AM +0200, Richard Biener wrote:
> On Wed, 5 Aug 2026, Aldy Hernandez wrote:
> 
> > On Wed, Aug 05, 2026 at 04:17:19PM +0200, Aldy Hernandez wrote:
> > > Per our discussion, nonzero_p() has always been a bit insane.  It only
> > > returns true for ~[0,0] so even [13,13] is considered false.
> > > Introduce contains_zero_p(), which from the looks of it, we've already
> > > been hand crafting, as direct nonzero_p() calls were few.
> > > 
> > > How does this look?
> > 
> > BTW, I ran my usual LAPACK source files through each patch
> > independently, and there are no changes to assembly, so in theory this
> > whole set is surprisingly a non-functional change.
> 
> You also add contains_zero_p for frange, but what does this actually
> mean there?  irange already had contains_zero_p.

I meant to remove the free-standing contains_zero_p(&irange) as a
follow-up, replacing all uses of it with r.contains_zero_p().

> I suppose we can document contains_zero_p to mean that
> for a value with the range val == T(0) may evaluate true?  So for
> frange this means either -0.0 or 0.0?  irange implements
> it in terms of contains_p which is already there for frange
> and that implements it as >= && <=.

Good call.  I'm adding:

+  // True if val == 0 may hold for some value in the range; for a float
+  // range that means +0.0 or -0.0.
+  virtual bool contains_zero_p () const = 0;

Thanks.
Aldy