diff mbox

[1/2,v3] PR77822

Message ID 20161121110352.GA16263@linux.vnet.ibm.com
State New
Headers show

Commit Message

Dominik Vogt Nov. 21, 2016, 11:03 a.m. UTC
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 ^_^  ^_^

Comments

Jeff Law Nov. 23, 2016, 8:22 p.m. UTC | #1
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
Dominik Vogt Nov. 24, 2016, 9:59 a.m. UTC | #2
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 ^_^  ^_^
Jeff Law Nov. 24, 2016, 3:33 p.m. UTC | #3
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
diff mbox

Patch

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.  */