[PATCH] Fix warnings building pdp11 port

[PATCH] Fix warnings building pdp11 port

[Jeff Law]

The pdp11 port fails to build with the trunk because of a warning. 
Essentially VRP determines that the result of using BRANCH_COST is a 
constant with the range [0..1].  That's always less than 4, 3 and the 
various other magic constants used with BRANCH_COST and VRP issues a 
warning about that comparison.

I expect we're going to be overhauling BRANCH_COST shortly.  In the mean 
time, this just revectors BRANCH_COST for the pdp11 into a function to 
prevent VRP from collapsing the test and issuing the warning.

Yes, this means more code in the pdp11 cross compiler.  I'm not terribly 
concerned about that and I couldn't stand the idea of scattering 
diagnostic push/pop stuff all over the place to make just the pdp11 port 
happy.


Tested by building the pdp11 targets from config-all.mk.

Installed on the trunk.

Jeff

Re: [PATCH] Fix warnings building pdp11 port

[Trevor Saunders]

On Tue, Sep 29, 2015 at 10:55:46AM -0600, Jeff Law wrote:
> The pdp11 port fails to build with the trunk because of a warning.
> Essentially VRP determines that the result of using BRANCH_COST is a
> constant with the range [0..1].  That's always less than 4, 3 and the
> various other magic constants used with BRANCH_COST and VRP issues a warning
> about that comparison.
> 
> I expect we're going to be overhauling BRANCH_COST shortly.  In the mean
> time, this just revectors BRANCH_COST for the pdp11 into a function to
> prevent VRP from collapsing the test and issuing the warning.
> 
> Yes, this means more code in the pdp11 cross compiler.  I'm not terribly
> concerned about that and I couldn't stand the idea of scattering diagnostic
> push/pop stuff all over the place to make just the pdp11 port happy.

ENOPATCH, but it seems like that's the right direction anyway since it
makes it slightly easier to convert the macro to a hook ;)

Trev

> 
> 
> Tested by building the pdp11 targets from config-all.mk.
> 
> Installed on the trunk.
> 
> Jeff

Re: [PATCH] Fix warnings building pdp11 port

[Jeff Law]

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

On 09/29/2015 12:11 PM, Trevor Saunders wrote:
> On Tue, Sep 29, 2015 at 10:55:46AM -0600, Jeff Law wrote:
>> The pdp11 port fails to build with the trunk because of a warning.
>> Essentially VRP determines that the result of using BRANCH_COST is a
>> constant with the range [0..1].  That's always less than 4, 3 and the
>> various other magic constants used with BRANCH_COST and VRP issues a warning
>> about that comparison.
>>
>> I expect we're going to be overhauling BRANCH_COST shortly.  In the mean
>> time, this just revectors BRANCH_COST for the pdp11 into a function to
>> prevent VRP from collapsing the test and issuing the warning.
>>
>> Yes, this means more code in the pdp11 cross compiler.  I'm not terribly
>> concerned about that and I couldn't stand the idea of scattering diagnostic
>> push/pop stuff all over the place to make just the pdp11 port happy.
>
> ENOPATCH, but it seems like that's the right direction anyway since it
> makes it slightly easier to convert the macro to a hook ;)
Bah.  Attached this time :-)

Yea, hookization was in the back of my mind when I made the final choice 
to use a function call.

Jeff

[-- Attachment #2: P --]
[-- Type: text/plain, Size: 2333 bytes --]

commit c6fc406c69342fdcca25ee48294bd43dd90facc2
Author: law <law@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Tue Sep 29 16:56:04 2015 +0000

    [PATCH] Fix warnings building pdp11 port
    
    	* config/pdp11/pdp11.c (pdp11_branch_cost): New function.
    	* config/pdp11/pdp11.h (BRANCH_COST): Call function rather than
    	inline macro expansion.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@228259 138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 68149c4..13e930a 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,5 +1,9 @@
 2015-09-29  Jeff Law  <law@redhat.com>
 
+	* config/pdp11/pdp11.c (pdp11_branch_cost): New function.
+	* config/pdp11/pdp11.h (BRANCH_COST): Call function rather than
+	inline macro expansion.
+
 	* config/i386/t-interix (winnt-stubs.o): Fix compilation rule.
 
 	* config/sh/sh.c (gen_shl_and): Fix undefined left shift
diff --git a/gcc/config/pdp11/pdp11-protos.h b/gcc/config/pdp11/pdp11-protos.h
index 86c6da3..aca3d82 100644
--- a/gcc/config/pdp11/pdp11-protos.h
+++ b/gcc/config/pdp11/pdp11-protos.h
@@ -47,3 +47,4 @@ extern void output_ascii (FILE *, const char *, int);
 extern void pdp11_asm_output_var (FILE *, const char *, int, int, bool);
 extern void pdp11_expand_prologue (void);
 extern void pdp11_expand_epilogue (void);
+extern int pdp11_branch_cost (void);
diff --git a/gcc/config/pdp11/pdp11.c b/gcc/config/pdp11/pdp11.c
index f0c2a5d..8eb37c6 100644
--- a/gcc/config/pdp11/pdp11.c
+++ b/gcc/config/pdp11/pdp11.c
@@ -1933,4 +1933,10 @@ pdp11_scalar_mode_supported_p (machine_mode mode)
   return default_scalar_mode_supported_p (mode);
 }
 
+int
+pdp11_branch_cost ()
+{
+  return (TARGET_BRANCH_CHEAP ? 0 : 1);
+}
+
 struct gcc_target targetm = TARGET_INITIALIZER;
diff --git a/gcc/config/pdp11/pdp11.h b/gcc/config/pdp11/pdp11.h
index 1d947f3..8339f1c 100644
--- a/gcc/config/pdp11/pdp11.h
+++ b/gcc/config/pdp11/pdp11.h
@@ -660,8 +660,7 @@ extern rtx cc0_reg_rtx;
 /* there is no point in avoiding branches on a pdp, 
    since branches are really cheap - I just want to find out
    how much difference the BRANCH_COST macro makes in code */
-#define BRANCH_COST(speed_p, predictable_p) (TARGET_BRANCH_CHEAP ? 0 : 1)
-
+#define BRANCH_COST(speed_p, predictable_p) pdp11_branch_cost ()
 
 #define COMPARE_FLAG_MODE HImode
 

Re: [PATCH] Fix warnings building pdp11 port

[Richard Biener]

On Tue, Sep 29, 2015 at 6:55 PM, Jeff Law <law@redhat.com> wrote:
> The pdp11 port fails to build with the trunk because of a warning.
> Essentially VRP determines that the result of using BRANCH_COST is a
> constant with the range [0..1].  That's always less than 4, 3 and the
> various other magic constants used with BRANCH_COST and VRP issues a warning
> about that comparison.

It does?  Huh.  Is it about undefined overflow which is the only thing
VRP should end up
warning about?  If so I wonder how that happens, at least I can't
reproduce it for
--target=pdp11 --enable-werror build of cc1.

> I expect we're going to be overhauling BRANCH_COST shortly.  In the mean
> time, this just revectors BRANCH_COST for the pdp11 into a function to
> prevent VRP from collapsing the test and issuing the warning.
>
> Yes, this means more code in the pdp11 cross compiler.  I'm not terribly
> concerned about that and I couldn't stand the idea of scattering diagnostic
> push/pop stuff all over the place to make just the pdp11 port happy.
>
>
> Tested by building the pdp11 targets from config-all.mk.
>
> Installed on the trunk.
>
> Jeff

Re: [PATCH] Fix warnings building pdp11 port

[Jeff Law]

On 09/30/2015 01:48 AM, Richard Biener wrote:
> On Tue, Sep 29, 2015 at 6:55 PM, Jeff Law <law@redhat.com> wrote:
>> The pdp11 port fails to build with the trunk because of a warning.
>> Essentially VRP determines that the result of using BRANCH_COST is a
>> constant with the range [0..1].  That's always less than 4, 3 and the
>> various other magic constants used with BRANCH_COST and VRP issues a warning
>> about that comparison.
>
> It does?  Huh.  Is it about undefined overflow which is the only thing
> VRP should end up
> warning about?  If so I wonder how that happens, at least I can't
> reproduce it for
> --target=pdp11 --enable-werror build of cc1.
You have to use a trunk compiler to build the pdp11 cross.  You'll bump 
into this repeatedly:

   if (warn_type_limits
       && ret && only_ranges
       && TREE_CODE_CLASS (code) == tcc_comparison
       && TREE_CODE (op0) == SSA_NAME)
     {
       /* If the comparison is being folded and the operand on the LHS
          is being compared against a constant value that is outside of
          the natural range of OP0's type, then the predicate will
          always fold regardless of the value of OP0.  If -Wtype-limits
          was specified, emit a warning.  */
       tree type = TREE_TYPE (op0);
       value_range_t *vr0 = get_value_range (op0);

       if (vr0->type == VR_RANGE
           && INTEGRAL_TYPE_P (type)
           && vrp_val_is_min (vr0->min)
           && vrp_val_is_max (vr0->max)
           && is_gimple_min_invariant (op1))
         {
           location_t location;

           if (!gimple_has_location (stmt))
             location = input_location;
           else
             location = gimple_location (stmt);

           warning_at (location, OPT_Wtype_limits,
                       integer_zerop (ret)
                       ? G_("comparison always false "
                            "due to limited range of data type")
                       : G_("comparison always true "
                            "due to limited range of data type"));
         }
     }

   return ret;
}


Jeff

Re: [PATCH] Fix warnings building pdp11 port

[Richard Biener]

On Wed, Sep 30, 2015 at 6:43 PM, Jeff Law <law@redhat.com> wrote:
> On 09/30/2015 01:48 AM, Richard Biener wrote:
>>
>> On Tue, Sep 29, 2015 at 6:55 PM, Jeff Law <law@redhat.com> wrote:
>>>
>>> The pdp11 port fails to build with the trunk because of a warning.
>>> Essentially VRP determines that the result of using BRANCH_COST is a
>>> constant with the range [0..1].  That's always less than 4, 3 and the
>>> various other magic constants used with BRANCH_COST and VRP issues a
>>> warning
>>> about that comparison.
>>
>>
>> It does?  Huh.  Is it about undefined overflow which is the only thing
>> VRP should end up
>> warning about?  If so I wonder how that happens, at least I can't
>> reproduce it for
>> --target=pdp11 --enable-werror build of cc1.
>
> You have to use a trunk compiler to build the pdp11 cross.  You'll bump into
> this repeatedly:
>
>   if (warn_type_limits
>       && ret && only_ranges
>       && TREE_CODE_CLASS (code) == tcc_comparison
>       && TREE_CODE (op0) == SSA_NAME)
>     {
>       /* If the comparison is being folded and the operand on the LHS
>          is being compared against a constant value that is outside of
>          the natural range of OP0's type, then the predicate will
>          always fold regardless of the value of OP0.  If -Wtype-limits
>          was specified, emit a warning.  */
>       tree type = TREE_TYPE (op0);
>       value_range_t *vr0 = get_value_range (op0);
>
>       if (vr0->type == VR_RANGE
>           && INTEGRAL_TYPE_P (type)
>           && vrp_val_is_min (vr0->min)
>           && vrp_val_is_max (vr0->max)
>           && is_gimple_min_invariant (op1))
>         {
>           location_t location;
>
>           if (!gimple_has_location (stmt))
>             location = input_location;
>           else
>             location = gimple_location (stmt);
>
>           warning_at (location, OPT_Wtype_limits,
>                       integer_zerop (ret)
>                       ? G_("comparison always false "
>                            "due to limited range of data type")
>                       : G_("comparison always true "
>                            "due to limited range of data type"));
>         }
>     }

Oh, I didn't remember we have this kind of warning in VRP ... it's
bound to trigger
for example after jump-threading.  So I'm not sure it's useful.

Richard.

>   return ret;
> }
>
>
> Jeff

Re: [PATCH] Fix warnings building pdp11 port

[Jeff Law]

On 10/01/2015 03:49 AM, Richard Biener wrote:
> On Wed, Sep 30, 2015 at 6:43 PM, Jeff Law <law@redhat.com> wrote:
>> On 09/30/2015 01:48 AM, Richard Biener wrote:
>>>
>>> On Tue, Sep 29, 2015 at 6:55 PM, Jeff Law <law@redhat.com> wrote:
>>>>
>>>> The pdp11 port fails to build with the trunk because of a warning.
>>>> Essentially VRP determines that the result of using BRANCH_COST is a
>>>> constant with the range [0..1].  That's always less than 4, 3 and the
>>>> various other magic constants used with BRANCH_COST and VRP issues a
>>>> warning
>>>> about that comparison.
>>>
>>>
>>> It does?  Huh.  Is it about undefined overflow which is the only thing
>>> VRP should end up
>>> warning about?  If so I wonder how that happens, at least I can't
>>> reproduce it for
>>> --target=pdp11 --enable-werror build of cc1.
>>
>> You have to use a trunk compiler to build the pdp11 cross.  You'll bump into
>> this repeatedly:
>>
>>    if (warn_type_limits
>>        && ret && only_ranges
>>        && TREE_CODE_CLASS (code) == tcc_comparison
>>        && TREE_CODE (op0) == SSA_NAME)
>>      {
>>        /* If the comparison is being folded and the operand on the LHS
>>           is being compared against a constant value that is outside of
>>           the natural range of OP0's type, then the predicate will
>>           always fold regardless of the value of OP0.  If -Wtype-limits
>>           was specified, emit a warning.  */
>>        tree type = TREE_TYPE (op0);
>>        value_range_t *vr0 = get_value_range (op0);
>>
>>        if (vr0->type == VR_RANGE
>>            && INTEGRAL_TYPE_P (type)
>>            && vrp_val_is_min (vr0->min)
>>            && vrp_val_is_max (vr0->max)
>>            && is_gimple_min_invariant (op1))
>>          {
>>            location_t location;
>>
>>            if (!gimple_has_location (stmt))
>>              location = input_location;
>>            else
>>              location = gimple_location (stmt);
>>
>>            warning_at (location, OPT_Wtype_limits,
>>                        integer_zerop (ret)
>>                        ? G_("comparison always false "
>>                             "due to limited range of data type")
>>                        : G_("comparison always true "
>>                             "due to limited range of data type"));
>>          }
>>      }
>
> Oh, I didn't remember we have this kind of warning in VRP ... it's
> bound to trigger
> for example after jump-threading.  So I'm not sure it's useful.
It caught me by surprise as well.  It's a poor man's attempt at 
unreachable code warnings.  Strangely, it's been around since 2009, but 
is only just now causing problems.  I'd certainly question it's utility 
as well.

That may be a symptom of something else not optimizing the condition 
earlier or we've made some changes that expose the collapsed range to VRP.

Jef