Re: [PATCH 1/2 v3] PR77822
On Fri, Nov 25, 2016 at 09:29:46AM +0100, Dominik Vogt wrote: > gcc/ChangeLog > > PR target/77822 > * rtl.h (EXTRACT_ARGS_IN_RANGE): New. Applied. Thanks! -Andreas-
Re: [PATCH 1/2 v3] PR77822
On 11/25/2016 01:29 AM, Dominik Vogt wrote:
On Thu, Nov 24, 2016 at 08:33:32AM -0700, Jeff Law wrote:
On 11/24/2016 02:59 AM, Dominik Vogt wrote:
On Wed, Nov 23, 2016 at 01:22:31PM -0700, Jeff Law wrote:
PR target/77822
* system.h (SIZE_POS_IN_RANGE): New.
OK. Though system.h seems like an unfortunate place. Would rtl.h
work better since this is really about verifying the arguments to
things like {zero,sign}_extract which are RTL concepts.
I was unsure whether it's better to give the macro a name not
related to *_extract and put it into system.h or make it specific
to extracts and put it in rtl.h.
Yea. What tends to tip the balance for me is that it's likely not
reusable outside extraction argument testing.
It's probably not really reuseable elsewhere, so when moving it to
rtl.h I'll also rename it to EXTRACT_ARGS_IN_RANGE. Okay?
Yea, that sounds perfect.
Version 4 of the patch attached. Bootstrapped and regression
tested on s390x biarch and s390. Changes since the last version:
v4:
Rename SIZE_POS_IN_RANGE to EXTRACT_ARGS_IN_RANGE and move
it to rtl.h.
OK for the trunk -- just in case you were waiting on an explicit ack of V4.
jeff
Re: [PATCH 1/2 v3] PR77822
On Thu, Nov 24, 2016 at 08:33:32AM -0700, Jeff Law wrote:
> On 11/24/2016 02:59 AM, Dominik Vogt wrote:
> >On Wed, Nov 23, 2016 at 01:22:31PM -0700, Jeff Law wrote:
> >>> PR target/77822
> >>> * system.h (SIZE_POS_IN_RANGE): New.
> >>OK. Though system.h seems like an unfortunate place. Would rtl.h
> >>work better since this is really about verifying the arguments to
> >>things like {zero,sign}_extract which are RTL concepts.
> >
> >I was unsure whether it's better to give the macro a name not
> >related to *_extract and put it into system.h or make it specific
> >to extracts and put it in rtl.h.
> Yea. What tends to tip the balance for me is that it's likely not
> reusable outside extraction argument testing.
>
> >
> >It's probably not really reuseable elsewhere, so when moving it to
> >rtl.h I'll also rename it to EXTRACT_ARGS_IN_RANGE. Okay?
> Yea, that sounds perfect.
Version 4 of the patch attached. Bootstrapped and regression
tested on s390x biarch and s390. Changes since the last version:
v4:
Rename SIZE_POS_IN_RANGE to EXTRACT_ARGS_IN_RANGE and move
it to rtl.h.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
gcc/ChangeLog
PR target/77822
* rtl.h (EXTRACT_ARGS_IN_RANGE): New.
>From 8e5bcea2983b50ba133a83086fcc21c31d8dbabe Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Thu, 17 Nov 2016 14:49:18 +0100
Subject: [PATCH 1/2] PR target/77822: Add helper macro EXTRACT_ARGS_IN_RANGE
to system.h.
The macro can be used to validate the arguments of zero_extract and
sign_extract to fix this problem:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77822
---
gcc/rtl.h | 10 ++
1 file changed, 10 insertions(+)
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 660d381..1847bce 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -2694,6 +2694,16 @@ get_full_set_src_cost (rtx x, machine_mode mode, struct
full_rtx_costs *c)
}
#endif
+/* A convenience macro to validate the arguments of a zero_extract
+ expression. It determines whether SIZE lies inclusively within
+ [1, RANGE], POS lies inclusively within between [0, RANGE - 1]
+ and the sum lies inclusively within [1, RANGE]. RANGE must be
+ >= 1, but SIZE and POS may be negative. */
+#define EXTRACT_ARGS_IN_RANGE(SIZE, POS, RANGE) \
+ (IN_RANGE ((POS), 0, (unsigned HOST_WIDE_INT) (RANGE) - 1) \
+ && IN_RANGE ((SIZE), 1, (unsigned HOST_WIDE_INT) (RANGE) \
+ - (unsigned HOST_WIDE_INT)(POS)))
+
/* In explow.c */
extern HOST_WIDE_INT trunc_int_for_mode(HOST_WIDE_INT, machine_mode);
extern rtx plus_constant (machine_mode, rtx, HOST_WIDE_INT, bool = false);
--
2.3.0
Re: [PATCH 1/2 v3] PR77822
On 11/24/2016 02:59 AM, Dominik Vogt wrote:
On Wed, Nov 23, 2016 at 01:22:31PM -0700, Jeff Law wrote:
On 11/21/2016 04:03 AM, Dominik Vogt wrote:
On Fri, Nov 18, 2016 at 04:29:18PM +0100, Dominik Vogt wrote:
On Fri, Nov 18, 2016 at 08:02:08AM -0600, Segher Boessenkool wrote:
On Fri, Nov 18, 2016 at 01:09:24PM +0100, Dominik Vogt wrote:
IN_RANGE(POS...) makes sure that POS is a non-negative number
smaller than UPPER, so (UPPER) - (POS) does not wrap. Or is there
some case that the new macro does not handle?
I think it works fine.
With a name like UPPER, you may want to have SIZE_POS_IN_RANGE work like
IN_RANGE, i.e. UPPER is inclusive then. Dunno.
Yeah, maybe rather call it RANGE to avoid too much similarity.
Some name that is so vague that one has to read the documentation.
I'll post an updated patch later.
Third version of the patch attached. The only difference is that
the macro argument name has been changed back to RANGE.
PR target/77822
* system.h (SIZE_POS_IN_RANGE): New.
OK. Though system.h seems like an unfortunate place. Would rtl.h
work better since this is really about verifying the arguments to
things like {zero,sign}_extract which are RTL concepts.
I was unsure whether it's better to give the macro a name not
related to *_extract and put it into system.h or make it specific
to extracts and put it in rtl.h.
Yea. What tends to tip the balance for me is that it's likely not
reusable outside extraction argument testing.
It's probably not really reuseable elsewhere, so when moving it to
rtl.h I'll also rename it to EXTRACT_ARGS_IN_RANGE. Okay?
Yea, that sounds perfect.
Jeff
Re: [PATCH 1/2 v3] PR77822
On Wed, Nov 23, 2016 at 01:22:31PM -0700, Jeff Law wrote:
> On 11/21/2016 04:03 AM, Dominik Vogt wrote:
> >On Fri, Nov 18, 2016 at 04:29:18PM +0100, Dominik Vogt wrote:
> >>> On Fri, Nov 18, 2016 at 08:02:08AM -0600, Segher Boessenkool wrote:
> > On Fri, Nov 18, 2016 at 01:09:24PM +0100, Dominik Vogt wrote:
> > > > IN_RANGE(POS...) makes sure that POS is a non-negative number
> > > > smaller than UPPER, so (UPPER) - (POS) does not wrap. Or is there
> > > > some case that the new macro does not handle?
> >
> > I think it works fine.
> >
> > With a name like UPPER, you may want to have SIZE_POS_IN_RANGE work
> > like
> > IN_RANGE, i.e. UPPER is inclusive then. Dunno.
> >>>
> >>> Yeah, maybe rather call it RANGE to avoid too much similarity.
> >>> Some name that is so vague that one has to read the documentation.
> >>> I'll post an updated patch later.
> >Third version of the patch attached. The only difference is that
> >the macro argument name has been changed back to RANGE.
> >
> > PR target/77822
> > * system.h (SIZE_POS_IN_RANGE): New.
> OK. Though system.h seems like an unfortunate place. Would rtl.h
> work better since this is really about verifying the arguments to
> things like {zero,sign}_extract which are RTL concepts.
I was unsure whether it's better to give the macro a name not
related to *_extract and put it into system.h or make it specific
to extracts and put it in rtl.h.
It's probably not really reuseable elsewhere, so when moving it to
rtl.h I'll also rename it to EXTRACT_ARGS_IN_RANGE. Okay?
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
Re: [PATCH 1/2 v3] PR77822
On 11/21/2016 04:03 AM, Dominik Vogt wrote:
On Fri, Nov 18, 2016 at 04:29:18PM +0100, Dominik Vogt wrote:
> On Fri, Nov 18, 2016 at 08:02:08AM -0600, Segher Boessenkool wrote:
> > On Fri, Nov 18, 2016 at 01:09:24PM +0100, Dominik Vogt wrote:
> > > IN_RANGE(POS...) makes sure that POS is a non-negative number
> > > smaller than UPPER, so (UPPER) - (POS) does not wrap. Or is there
> > > some case that the new macro does not handle?
> >
> > I think it works fine.
> >
> > With a name like UPPER, you may want to have SIZE_POS_IN_RANGE work like
> > IN_RANGE, i.e. UPPER is inclusive then. Dunno.
>
> Yeah, maybe rather call it RANGE to avoid too much similarity.
> Some name that is so vague that one has to read the documentation.
> I'll post an updated patch later.
Third version of the patch attached. The only difference is that
the macro argument name has been changed back to RANGE.
Ciao
Dominik ^_^ ^_^
-- Dominik Vogt IBM Germany
0001-v3-ChangeLog
gcc/ChangeLog
PR target/77822
* system.h (SIZE_POS_IN_RANGE): New.
OK. Though system.h seems like an unfortunate place. Would rtl.h work
better since this is really about verifying the arguments to things like
{zero,sign}_extract which are RTL concepts.
jeff
Re: [PATCH 1/2 v3] PR77822
On Fri, Nov 18, 2016 at 04:29:18PM +0100, Dominik Vogt wrote: > On Fri, Nov 18, 2016 at 08:02:08AM -0600, Segher Boessenkool wrote: > > On Fri, Nov 18, 2016 at 01:09:24PM +0100, Dominik Vogt wrote: > > > IN_RANGE(POS...) makes sure that POS is a non-negative number > > > smaller than UPPER, so (UPPER) - (POS) does not wrap. Or is there > > > some case that the new macro does not handle? > > > > I think it works fine. > > > > With a name like UPPER, you may want to have SIZE_POS_IN_RANGE work like > > IN_RANGE, i.e. UPPER is inclusive then. Dunno. > > Yeah, maybe rather call it RANGE to avoid too much similarity. > Some name that is so vague that one has to read the documentation. > I'll post an updated patch later. Third version of the patch attached. The only difference is that the macro argument name has been changed back to RANGE. Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany gcc/ChangeLog PR target/77822 * system.h (SIZE_POS_IN_RANGE): New. >From 8e02352c70d478c74155d5d127560da31363dd8a Mon Sep 17 00:00:00 2001 From: Dominik Vogt Date: Thu, 17 Nov 2016 14:49:18 +0100 Subject: [PATCH 1/2] PR target/77822: Add helper macro SIZE_POS_IN_RANGE to system.h. The macro can be used to validate the arguments of zero_extract and sign_extract to fix this problem: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77822 --- gcc/system.h | 9 + 1 file changed, 9 insertions(+) diff --git a/gcc/system.h b/gcc/system.h index 8c6127c..6c1228d 100644 --- a/gcc/system.h +++ b/gcc/system.h @@ -316,6 +316,15 @@ extern int errno; ((unsigned HOST_WIDE_INT) (VALUE) - (unsigned HOST_WIDE_INT) (LOWER) \ <= (unsigned HOST_WIDE_INT) (UPPER) - (unsigned HOST_WIDE_INT) (LOWER)) +/* A convenience macro to determine whether SIZE lies inclusively + within [1, RANGE], POS lies inclusively within between + [0, RANGE - 1] and the sum lies inclusively within [1, RANGE]. + RANGE must be >= 1, but SIZE and POS may be negative. */ +#define SIZE_POS_IN_RANGE(SIZE, POS, RANGE) \ + (IN_RANGE ((POS), 0, (unsigned HOST_WIDE_INT) (RANGE) - 1) \ + && IN_RANGE ((SIZE), 1, (unsigned HOST_WIDE_INT) (RANGE) \ + - (unsigned HOST_WIDE_INT)(POS))) + /* Infrastructure for defining missing _MAX and _MIN macros. Note that macros defined with these cannot be used in #if. */ -- 2.3.0
