On 10/22/20 3:16 PM, Andrew MacLeod via Gcc-patches wrote:
On 10/22/20 12:53 PM, Eric Botcazou wrote:
There are a few places in the ranger where we sanity check the types of
the ranges.  We were using types_compatible_p() but thats not really
acccurate as gimple allows types which are useless_type_conversion_p()
in only one direction, whereas types_compatible_p() requires casts in
both directions to be useless_type_conversion_p().

And, since its only a sanity check, ranges really only require that the
precision and sign be the same, so its a faster check anyway.

bootstrapped on x86_64-pc-linux-gnu, no regressions, pushed.
The Ada regression comes from this hunk:

diff --git a/gcc/gimple-range-gori.cc b/gcc/gimple-range-gori.cc
index c4bfc658319..983f4c97e87 100644
--- a/gcc/gimple-range-gori.cc
+++ b/gcc/gimple-range-gori.cc
@@ -552,7 +552,7 @@ is_gimple_logical_p (const gimple *gs)
      case BIT_AND_EXPR:
      case BIT_IOR_EXPR:
        // Bitwise operations on single bits are logical too.
-      if (types_compatible_p (TREE_TYPE (gimple_assign_rhs1 (gs)),
+      if (range_compatible_p (TREE_TYPE (gimple_assign_rhs1 (gs)),
                    boolean_type_node))
          return true;
        break;

which overlooks that boolean_type_node may have a precision not equal to 1
(it's 8 in Ada).  See useless_type_conversion_p which has:

       /* Preserve conversions to/from BOOLEAN_TYPE if types are not
     of precision one.  */
       if (((TREE_CODE (inner_type) == BOOLEAN_TYPE)
       != (TREE_CODE (outer_type) == BOOLEAN_TYPE))
      && TYPE_PRECISION (outer_type) != 1)
    return false

bah.  And I cant seem to reproduce it on my machine with your reproducer, but I am seeing the other result it in my log. Point is still taken tho. range_compatible_p should only be used during asserts as a sanity check for range interactions, not during actual code processing like that.

im currently testing the following, which reverts 2 places (both which check for logicals)  to use types_compatible_p instead.  The remaining uses are in range assertion code. This seems to resolve the original problem in my log.

Thanks for reducing it to the problematic change.  I'm verifying a new failure in libgomp isnt a result of this before I check it in.

Andrew


Sorry for missing the regression.. it was there, it just snuck by me in the noise :-P

THis seems to resolve the issue on my end, and its the right thing.

Bootstrapped on x86_64-pc-linux-gnu, no regressions, for SURE this time, pushed.





Author: Andrew MacLeod <amacl...@redhat.com>
Date:   Thu Oct 22 15:39:37 2020 -0400

    Use precision and sign to compare types for ranges
    
    Updated to only use range_compatible_p in range assert sanity checks,
    not for actual type cmpatibility.
    
            * gimple-range-gori.cc (is_gimple_logical_p): Use types_compatible_p
            for logical compatibility.
            (logical_stmt_cache::cacheable_p): Ditto.

diff --git a/gcc/gimple-range-gori.cc b/gcc/gimple-range-gori.cc
index 983f4c97e87..5d50b111d2a 100644
--- a/gcc/gimple-range-gori.cc
+++ b/gcc/gimple-range-gori.cc
@@ -552,7 +552,7 @@ is_gimple_logical_p (const gimple *gs)
 	case BIT_AND_EXPR:
 	case BIT_IOR_EXPR:
 	  // Bitwise operations on single bits are logical too.
-	  if (range_compatible_p (TREE_TYPE (gimple_assign_rhs1 (gs)),
+	  if (types_compatible_p (TREE_TYPE (gimple_assign_rhs1 (gs)),
 				  boolean_type_node))
 	    return true;
 	  break;
@@ -1165,7 +1165,7 @@ bool
 logical_stmt_cache::cacheable_p (gimple *stmt, const irange *lhs_range) const
 {
   if (gimple_code (stmt) == GIMPLE_ASSIGN
-      && range_compatible_p (TREE_TYPE (gimple_assign_lhs (stmt)),
+      && types_compatible_p (TREE_TYPE (gimple_assign_lhs (stmt)),
 			     boolean_type_node)
       && TREE_CODE (gimple_assign_rhs1 (stmt)) == SSA_NAME)
     {

Reply via email to