* [PATCH] Fix warnings building pdp11 port @ 2015-09-29 18:11 Jeff Law 2015-09-29 18:36 ` Trevor Saunders 2015-09-30 8:14 ` Richard Biener 0 siblings, 2 replies; 7+ messages in thread From: Jeff Law @ 2015-09-29 18:11 UTC (permalink / raw) To: gcc-patches@gcc.gnu.org >> gcc-patches 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix warnings building pdp11 port 2015-09-29 18:11 [PATCH] Fix warnings building pdp11 port Jeff Law @ 2015-09-29 18:36 ` Trevor Saunders 2015-09-29 18:54 ` Jeff Law 2015-09-30 8:14 ` Richard Biener 1 sibling, 1 reply; 7+ messages in thread From: Trevor Saunders @ 2015-09-29 18:36 UTC (permalink / raw) To: Jeff Law; +Cc: gcc-patches@gcc.gnu.org >> gcc-patches 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix warnings building pdp11 port 2015-09-29 18:36 ` Trevor Saunders @ 2015-09-29 18:54 ` Jeff Law 0 siblings, 0 replies; 7+ messages in thread From: Jeff Law @ 2015-09-29 18:54 UTC (permalink / raw) To: Trevor Saunders; +Cc: gcc-patches@gcc.gnu.org >> gcc-patches [-- 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix warnings building pdp11 port 2015-09-29 18:11 [PATCH] Fix warnings building pdp11 port Jeff Law 2015-09-29 18:36 ` Trevor Saunders @ 2015-09-30 8:14 ` Richard Biener 2015-09-30 17:44 ` Jeff Law 1 sibling, 1 reply; 7+ messages in thread From: Richard Biener @ 2015-09-30 8:14 UTC (permalink / raw) To: Jeff Law; +Cc: gcc-patches@gcc.gnu.org >> gcc-patches 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix warnings building pdp11 port 2015-09-30 8:14 ` Richard Biener @ 2015-09-30 17:44 ` Jeff Law 2015-10-01 9:49 ` Richard Biener 0 siblings, 1 reply; 7+ messages in thread From: Jeff Law @ 2015-09-30 17:44 UTC (permalink / raw) To: Richard Biener; +Cc: gcc-patches@gcc.gnu.org >> gcc-patches 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix warnings building pdp11 port 2015-09-30 17:44 ` Jeff Law @ 2015-10-01 9:49 ` Richard Biener 2015-10-01 15:47 ` Jeff Law 0 siblings, 1 reply; 7+ messages in thread From: Richard Biener @ 2015-10-01 9:49 UTC (permalink / raw) To: Jeff Law; +Cc: gcc-patches@gcc.gnu.org >> gcc-patches 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix warnings building pdp11 port 2015-10-01 9:49 ` Richard Biener @ 2015-10-01 15:47 ` Jeff Law 0 siblings, 0 replies; 7+ messages in thread From: Jeff Law @ 2015-10-01 15:47 UTC (permalink / raw) To: Richard Biener; +Cc: gcc-patches@gcc.gnu.org >> gcc-patches 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-10-01 15:47 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-09-29 18:11 [PATCH] Fix warnings building pdp11 port Jeff Law 2015-09-29 18:36 ` Trevor Saunders 2015-09-29 18:54 ` Jeff Law 2015-09-30 8:14 ` Richard Biener 2015-09-30 17:44 ` Jeff Law 2015-10-01 9:49 ` Richard Biener 2015-10-01 15:47 ` Jeff Law
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).