Re: [PATCH 2/7] Do not query SCEV in range_of_phi unless dominators are available.

[Aldy Hernandez via Gcc-patches]

On 9/22/21 10:05 AM, Richard Biener wrote:
On Tue, Sep 21, 2021 at 7:17 PM Aldy Hernandez via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:



On 9/21/21 7:05 PM, Andrew MacLeod wrote:
On 9/21/21 12:53 PM, Aldy Hernandez wrote:
SCEV won't work without dominators and we can get called without
dominators from debug_ranger.

Another option would be to rename scev_initialized_p to something like
scev_available_p and move the check there.  For now, this will do.

Committed.

gcc/ChangeLog:

     * gimple-range-fold.cc (fold_using_range::range_of_phi): Check
     dom_info_available_p.
---
   gcc/gimple-range-fold.cc | 4 +++-
   1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
index 997d02dd4b9..4dbf4188ec2 100644
--- a/gcc/gimple-range-fold.cc
+++ b/gcc/gimple-range-fold.cc
@@ -781,7 +781,9 @@ fold_using_range::range_of_phi (irange &r, gphi
*phi, fur_source &src)
       }
     // If SCEV is available, query if this PHI has any knonwn values.
-  if (scev_initialized_p () && !POINTER_TYPE_P (TREE_TYPE (phi_def)))
+  if (dom_info_available_p (CDI_DOMINATORS)
+      && scev_initialized_p ()
+      && !POINTER_TYPE_P (TREE_TYPE (phi_def)))
       {
         value_range loop_range;
         class loop *l = loop_containing_stmt (phi);

Im confused.. if scev doesn't work without dominators, how is
scev_initialized_p() true if there are no dominators?     Are we
initializing it somewhere without dominators?  Maybe there should be a
check in the scev init routine?  seems like something else is amok.

As I mentioned, this can happen from debug_ranger(), which is a
debugging construct, and I've been known to call it without dominators
:).  And yes, I agree we could move it to scev_initialized_p.

But why is scev_initialized_p () true from debug_ranger()?

This seems to be an artifact of some internal diagnostic code I had 
comparing the new work with the ASSERT_EXPR threads.  I've reverted the 
patch.

Sorry for the noise, and thanks for pointing it out.
Aldy
commit e7797033a8f2bd9bcf455d796178f5c14d457513
Author: Aldy Hernandez <al...@redhat.com>
Date:   Thu Sep 23 09:40:59 2021 +0200

    Remove dominator check in fold_using_range::range_of_phi.
    
    Revert the following patch, as it was an artifact of diagnostic code
    being run with improper IL.
    
    commit 64b80b8819f9ea74712625bceb0ec4388e25f67d
    Author: Aldy Hernandez <al...@redhat.com>
    Date:   Tue Sep 21 08:28:28 2021 +0200
    
        Do not query SCEV in range_of_phi unless dominators are available.
    
        SCEV won't work without dominators and we can get called without
        dominators from debug_ranger.
    
    gcc/ChangeLog:
    
            * gimple-range-fold.cc (fold_using_range::range_of_phi):
            Remove dominator check.

diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
index 1da1befa9a2..35324fd72c2 100644
--- a/gcc/gimple-range-fold.cc
+++ b/gcc/gimple-range-fold.cc
@@ -826,9 +826,7 @@ fold_using_range::range_of_phi (irange &r, gphi *phi, fur_source &src)
       }
 
   // If SCEV is available, query if this PHI has any knonwn values.
-  if (dom_info_available_p (CDI_DOMINATORS)
-      && scev_initialized_p ()
-      && !POINTER_TYPE_P (TREE_TYPE (phi_def)))
+  if (scev_initialized_p () && !POINTER_TYPE_P (TREE_TYPE (phi_def)))
     {
       value_range loop_range;
       class loop *l = loop_containing_stmt (phi);