* RFC - VRP1 default mode
@ 2022-10-26 14:24 Andrew MacLeod
2022-10-26 14:59 ` Jeff Law
2022-10-28 7:17 ` Richard Biener
0 siblings, 2 replies; 9+ messages in thread
From: Andrew MacLeod @ 2022-10-26 14:24 UTC (permalink / raw)
To: gcc-patches; +Cc: hernandez, aldy, Richard Biener, Jakub Jelinek, Jeff Law
Figured I would ask what you guys think of making ranger the default for
the VRP1 pass now.
With partial equivalences and the other bits I checked in the past few
weeks I'm not aware of much that the legacy VRP pass gets that ranger
doesn't. The only exception to that which I am aware of is the trick
played with the unreachable edges to set global ranges, but that is done
in the DOM passes now anyway... so it just happens slightly later in the
optimization cycle. There is one test case that needs adjustment for
that which was just checking for a mask in DOM2
(gcc.dg/tree-ssa/pr107009.c). At this point I have not aware of
anything that Id be concerned about, and the testsuite seems to run
cleanly.
We could change the default now and see if any issues show up, giving us
a chance to address them. The code base has been well exercised for a
while so risk should be low. We could also reduce code size by
stripping out unneeded code if we so desired.
Or we could leave things as they are for one more cycle. My preference
would be to make the switch now and let it play out. Thoughts?
Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: RFC - VRP1 default mode
2022-10-26 14:24 RFC - VRP1 default mode Andrew MacLeod
@ 2022-10-26 14:59 ` Jeff Law
2022-10-28 7:17 ` Richard Biener
1 sibling, 0 replies; 9+ messages in thread
From: Jeff Law @ 2022-10-26 14:59 UTC (permalink / raw)
To: Andrew MacLeod, gcc-patches
Cc: hernandez, aldy, Richard Biener, Jakub Jelinek
On 10/26/22 08:24, Andrew MacLeod wrote:
> Figured I would ask what you guys think of making ranger the default
> for the VRP1 pass now.
>
> With partial equivalences and the other bits I checked in the past few
> weeks I'm not aware of much that the legacy VRP pass gets that ranger
> doesn't. The only exception to that which I am aware of is the trick
> played with the unreachable edges to set global ranges, but that is
> done in the DOM passes now anyway... so it just happens slightly later
> in the optimization cycle. There is one test case that needs
> adjustment for that which was just checking for a mask in DOM2
> (gcc.dg/tree-ssa/pr107009.c). At this point I have not aware of
> anything that Id be concerned about, and the testsuite seems to run
> cleanly.
>
> We could change the default now and see if any issues show up, giving
> us a chance to address them. The code base has been well exercised for
> a while so risk should be low. We could also reduce code size by
> stripping out unneeded code if we so desired.
>
> Or we could leave things as they are for one more cycle. My
> preference would be to make the switch now and let it play out. Thoughts?
I'd rather switch now than wait another cycle. We've got mass rebuilds
coming up in a few months which should give us a good indicator if
there's any notable issues that need to get resolved.
Jeff
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: RFC - VRP1 default mode
2022-10-26 14:24 RFC - VRP1 default mode Andrew MacLeod
2022-10-26 14:59 ` Jeff Law
@ 2022-10-28 7:17 ` Richard Biener
2022-10-28 13:43 ` Andrew MacLeod
1 sibling, 1 reply; 9+ messages in thread
From: Richard Biener @ 2022-10-28 7:17 UTC (permalink / raw)
To: Andrew MacLeod; +Cc: gcc-patches, hernandez, aldy, Jakub Jelinek, Jeff Law
On Wed, Oct 26, 2022 at 4:24 PM Andrew MacLeod <amacleod@redhat.com> wrote:
>
> Figured I would ask what you guys think of making ranger the default for
> the VRP1 pass now.
>
> With partial equivalences and the other bits I checked in the past few
> weeks I'm not aware of much that the legacy VRP pass gets that ranger
> doesn't. The only exception to that which I am aware of is the trick
> played with the unreachable edges to set global ranges, but that is done
> in the DOM passes now anyway... so it just happens slightly later in the
> optimization cycle.
Note DOM should go away at some point. Why can this not happen during
ranger driven VRP?
> There is one test case that needs adjustment for
> that which was just checking for a mask in DOM2
> (gcc.dg/tree-ssa/pr107009.c). At this point I have not aware of
> anything that Id be concerned about, and the testsuite seems to run
> cleanly.
Did you enable Ada? The only feature I don't see implemented is
symbolic range handling which boils down to general base + constant offset
range endpoints (that's what symbolic ranges allow). That area was
specifically improved to optimize range checks emitted by the Ada frontend
but IIRC also applies to fortran -frange-check (not sure about test coverage
of that).
> We could change the default now and see if any issues show up, giving us
> a chance to address them. The code base has been well exercised for a
> while so risk should be low. We could also reduce code size by
> stripping out unneeded code if we so desired.
>
> Or we could leave things as they are for one more cycle. My preference
> would be to make the switch now and let it play out. Thoughts?
>
> Andrew
>
>
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: RFC - VRP1 default mode
2022-10-28 7:17 ` Richard Biener
@ 2022-10-28 13:43 ` Andrew MacLeod
2022-10-28 13:46 ` Richard Biener
2022-10-28 17:45 ` Eric Botcazou
0 siblings, 2 replies; 9+ messages in thread
From: Andrew MacLeod @ 2022-10-28 13:43 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches, hernandez, aldy, Jakub Jelinek, Jeff Law
On 10/28/22 03:17, Richard Biener wrote:
> On Wed, Oct 26, 2022 at 4:24 PM Andrew MacLeod <amacleod@redhat.com> wrote:
>> Figured I would ask what you guys think of making ranger the default for
>> the VRP1 pass now.
>>
>> With partial equivalences and the other bits I checked in the past few
>> weeks I'm not aware of much that the legacy VRP pass gets that ranger
>> doesn't. The only exception to that which I am aware of is the trick
>> played with the unreachable edges to set global ranges, but that is done
>> in the DOM passes now anyway... so it just happens slightly later in the
>> optimization cycle.
> Note DOM should go away at some point. Why can this not happen during
> ranger driven VRP?
I have been working on that for the last 2 days. Turns out VRP1 can
remove builtin_unreachable from the
if (X)
__builtin_unreachable ()
idiom and set the appropriate global ranges, but it has to leave those
with 2 ssa-names:
if (a_1 != b_2)
__builtin_unreachable()
until the second pass of VRP or we lose the relationship between a_1 and
b_2. That triggers some failures. Specifically a vectorizor fail
because it cant be sure that the start and end point are not the same
without the condition in the IL. Trying to store global relations over
multiple passes would be problematic at this stage of development, so I
don't see a problem with leaving it that way.
bultin_unreachables() from switches get removed during the second pass
of switch-conversion... which I presume remains OK.
Anyway, thats pretty much under control. Patch probably coming later today.
>> There is one test case that needs adjustment for
>> that which was just checking for a mask in DOM2
>> (gcc.dg/tree-ssa/pr107009.c). At this point I have not aware of
>> anything that Id be concerned about, and the testsuite seems to run
>> cleanly.
> Did you enable Ada? The only feature I don't see implemented is
> symbolic range handling which boils down to general base + constant offset
> range endpoints (that's what symbolic ranges allow). That area was
> specifically improved to optimize range checks emitted by the Ada frontend
> but IIRC also applies to fortran -frange-check (not sure about test coverage
> of that).
I get a clean testsuite run configured and bootstrapped with
--enable-languages=c,c++,go,fortran,ada,obj-c++,jit --enable-host-shared
Is there a PR or specific tests in either fortran or ada for those
improvements? ie, something specific I should check for? Part of rangers
point is to be able to do symbolic relationships without storing the
symbolic in the range, just picking it up from the IL as needed.
Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: RFC - VRP1 default mode
2022-10-28 13:43 ` Andrew MacLeod
@ 2022-10-28 13:46 ` Richard Biener
2022-10-28 13:59 ` Andrew MacLeod
2022-10-28 17:45 ` Eric Botcazou
1 sibling, 1 reply; 9+ messages in thread
From: Richard Biener @ 2022-10-28 13:46 UTC (permalink / raw)
To: Andrew MacLeod, Eric Botcazou
Cc: gcc-patches, hernandez, aldy, Jakub Jelinek, Jeff Law
On Fri, Oct 28, 2022 at 3:43 PM Andrew MacLeod <amacleod@redhat.com> wrote:
>
>
> On 10/28/22 03:17, Richard Biener wrote:
> > On Wed, Oct 26, 2022 at 4:24 PM Andrew MacLeod <amacleod@redhat.com> wrote:
> >> Figured I would ask what you guys think of making ranger the default for
> >> the VRP1 pass now.
> >>
> >> With partial equivalences and the other bits I checked in the past few
> >> weeks I'm not aware of much that the legacy VRP pass gets that ranger
> >> doesn't. The only exception to that which I am aware of is the trick
> >> played with the unreachable edges to set global ranges, but that is done
> >> in the DOM passes now anyway... so it just happens slightly later in the
> >> optimization cycle.
> > Note DOM should go away at some point. Why can this not happen during
> > ranger driven VRP?
>
> I have been working on that for the last 2 days. Turns out VRP1 can
> remove builtin_unreachable from the
> if (X)
> __builtin_unreachable ()
>
> idiom and set the appropriate global ranges, but it has to leave those
> with 2 ssa-names:
>
> if (a_1 != b_2)
> __builtin_unreachable()
>
> until the second pass of VRP or we lose the relationship between a_1 and
> b_2. That triggers some failures. Specifically a vectorizor fail
> because it cant be sure that the start and end point are not the same
> without the condition in the IL. Trying to store global relations over
> multiple passes would be problematic at this stage of development, so I
> don't see a problem with leaving it that way.
Hmm, I don't remember VRP1 doing anything special with the above though?
Did it somehow propagate the (un!)conditional equivalence?
> bultin_unreachables() from switches get removed during the second pass
> of switch-conversion... which I presume remains OK.
>
> Anyway, thats pretty much under control. Patch probably coming later today.
>
>
>
> >> There is one test case that needs adjustment for
> >> that which was just checking for a mask in DOM2
> >> (gcc.dg/tree-ssa/pr107009.c). At this point I have not aware of
> >> anything that Id be concerned about, and the testsuite seems to run
> >> cleanly.
> > Did you enable Ada? The only feature I don't see implemented is
> > symbolic range handling which boils down to general base + constant offset
> > range endpoints (that's what symbolic ranges allow). That area was
> > specifically improved to optimize range checks emitted by the Ada frontend
> > but IIRC also applies to fortran -frange-check (not sure about test coverage
> > of that).
> I get a clean testsuite run configured and bootstrapped with
>
> --enable-languages=c,c++,go,fortran,ada,obj-c++,jit --enable-host-shared
>
> Is there a PR or specific tests in either fortran or ada for those
> improvements? ie, something specific I should check for? Part of rangers
> point is to be able to do symbolic relationships without storing the
> symbolic in the range, just picking it up from the IL as needed.
I'm defering to Eric here.
Richard.
> Andrew
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: RFC - VRP1 default mode
2022-10-28 13:46 ` Richard Biener
@ 2022-10-28 13:59 ` Andrew MacLeod
2022-10-28 14:14 ` Richard Biener
0 siblings, 1 reply; 9+ messages in thread
From: Andrew MacLeod @ 2022-10-28 13:59 UTC (permalink / raw)
To: Richard Biener, Eric Botcazou
Cc: gcc-patches, hernandez, aldy, Jakub Jelinek, Jeff Law
On 10/28/22 09:46, Richard Biener wrote:
> On Fri, Oct 28, 2022 at 3:43 PM Andrew MacLeod <amacleod@redhat.com> wrote:
>>
>> On 10/28/22 03:17, Richard Biener wrote:
>>> On Wed, Oct 26, 2022 at 4:24 PM Andrew MacLeod <amacleod@redhat.com> wrote:
>>>> Figured I would ask what you guys think of making ranger the default for
>>>> the VRP1 pass now.
>>>>
>>>> With partial equivalences and the other bits I checked in the past few
>>>> weeks I'm not aware of much that the legacy VRP pass gets that ranger
>>>> doesn't. The only exception to that which I am aware of is the trick
>>>> played with the unreachable edges to set global ranges, but that is done
>>>> in the DOM passes now anyway... so it just happens slightly later in the
>>>> optimization cycle.
>>> Note DOM should go away at some point. Why can this not happen during
>>> ranger driven VRP?
>> I have been working on that for the last 2 days. Turns out VRP1 can
>> remove builtin_unreachable from the
>> if (X)
>> __builtin_unreachable ()
>>
>> idiom and set the appropriate global ranges, but it has to leave those
>> with 2 ssa-names:
>>
>> if (a_1 != b_2)
>> __builtin_unreachable()
>>
>> until the second pass of VRP or we lose the relationship between a_1 and
>> b_2. That triggers some failures. Specifically a vectorizor fail
>> because it cant be sure that the start and end point are not the same
>> without the condition in the IL. Trying to store global relations over
>> multiple passes would be problematic at this stage of development, so I
>> don't see a problem with leaving it that way.
> Hmm, I don't remember VRP1 doing anything special with the above though?
> Did it somehow propagate the (un!)conditional equivalence?
So as I looked at builtin_unreachable(), it was very adhoc. That one of
the roots of that artificial testcase in the PR I opened. Cascading
calls were not being handled in a consistent way. VRP1 removed some, dom
removed some.. they just kind of disappeared at some point, but not
consistently. The PR that Uli opened that Aldy fixed, I could make fail
again with minor adjustments to the conditions. So I worked on a
consistent approach.
My guess is the old range stored globally for that case for a_1 was
probably ~[b_2, b_2] meaning it was carried in the range. Until we have
an overall global relation tracker, we can't represent that across passes.
It appears that leaving those until VRP2 works fine... testsuite
currently running tho ;-)
Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: RFC - VRP1 default mode
2022-10-28 13:59 ` Andrew MacLeod
@ 2022-10-28 14:14 ` Richard Biener
2022-10-28 14:33 ` Andrew MacLeod
0 siblings, 1 reply; 9+ messages in thread
From: Richard Biener @ 2022-10-28 14:14 UTC (permalink / raw)
To: Andrew MacLeod
Cc: Eric Botcazou, gcc-patches, hernandez, aldy, Jakub Jelinek, Jeff Law
> Am 28.10.2022 um 15:59 schrieb Andrew MacLeod <amacleod@redhat.com>:
>
>
>> On 10/28/22 09:46, Richard Biener wrote:
>>> On Fri, Oct 28, 2022 at 3:43 PM Andrew MacLeod <amacleod@redhat.com> wrote:
>>>
>>> On 10/28/22 03:17, Richard Biener wrote:
>>>> On Wed, Oct 26, 2022 at 4:24 PM Andrew MacLeod <amacleod@redhat.com> wrote:
>>>>> Figured I would ask what you guys think of making ranger the default for
>>>>> the VRP1 pass now.
>>>>>
>>>>> With partial equivalences and the other bits I checked in the past few
>>>>> weeks I'm not aware of much that the legacy VRP pass gets that ranger
>>>>> doesn't. The only exception to that which I am aware of is the trick
>>>>> played with the unreachable edges to set global ranges, but that is done
>>>>> in the DOM passes now anyway... so it just happens slightly later in the
>>>>> optimization cycle.
>>>> Note DOM should go away at some point. Why can this not happen during
>>>> ranger driven VRP?
>>> I have been working on that for the last 2 days. Turns out VRP1 can
>>> remove builtin_unreachable from the
>>> if (X)
>>> __builtin_unreachable ()
>>>
>>> idiom and set the appropriate global ranges, but it has to leave those
>>> with 2 ssa-names:
>>>
>>> if (a_1 != b_2)
>>> __builtin_unreachable()
>>>
>>> until the second pass of VRP or we lose the relationship between a_1 and
>>> b_2. That triggers some failures. Specifically a vectorizor fail
>>> because it cant be sure that the start and end point are not the same
>>> without the condition in the IL. Trying to store global relations over
>>> multiple passes would be problematic at this stage of development, so I
>>> don't see a problem with leaving it that way.
>> Hmm, I don't remember VRP1 doing anything special with the above though?
>> Did it somehow propagate the (un!)conditional equivalence?
>
> So as I looked at builtin_unreachable(), it was very adhoc. That one of the roots of that artificial testcase in the PR I opened. Cascading calls were not being handled in a consistent way. VRP1 removed some, dom removed some.. they just kind of disappeared at some point, but not consistently. The PR that Uli opened that Aldy fixed, I could make fail again with minor adjustments to the conditions. So I worked on a consistent approach.
>
> My guess is the old range stored globally for that case for a_1 was probably ~[b_2, b_2] meaning it was carried in the range. Until we have an overall global relation tracker, we can't represent that across passes.
The global ranges were never symbolic, this was at most used during VRP itself.
>
> It appears that leaving those until VRP2 works fine... testsuite currently running tho ;-)
>
> Andrew
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: RFC - VRP1 default mode
2022-10-28 14:14 ` Richard Biener
@ 2022-10-28 14:33 ` Andrew MacLeod
0 siblings, 0 replies; 9+ messages in thread
From: Andrew MacLeod @ 2022-10-28 14:33 UTC (permalink / raw)
To: Richard Biener
Cc: Eric Botcazou, gcc-patches, hernandez, aldy, Jakub Jelinek, Jeff Law
On 10/28/22 10:14, Richard Biener wrote:
>
>> Am 28.10.2022 um 15:59 schrieb Andrew MacLeod <amacleod@redhat.com>:
>>
>>
>>> On 10/28/22 09:46, Richard Biener wrote:
>>>> On Fri, Oct 28, 2022 at 3:43 PM Andrew MacLeod <amacleod@redhat.com> wrote:
>>>>
>>>> On 10/28/22 03:17, Richard Biener wrote:
>>>>> On Wed, Oct 26, 2022 at 4:24 PM Andrew MacLeod <amacleod@redhat.com> wrote:
>>>>>> Figured I would ask what you guys think of making ranger the default for
>>>>>> the VRP1 pass now.
>>>>>>
>>>>>> With partial equivalences and the other bits I checked in the past few
>>>>>> weeks I'm not aware of much that the legacy VRP pass gets that ranger
>>>>>> doesn't. The only exception to that which I am aware of is the trick
>>>>>> played with the unreachable edges to set global ranges, but that is done
>>>>>> in the DOM passes now anyway... so it just happens slightly later in the
>>>>>> optimization cycle.
>>>>> Note DOM should go away at some point. Why can this not happen during
>>>>> ranger driven VRP?
>>>> I have been working on that for the last 2 days. Turns out VRP1 can
>>>> remove builtin_unreachable from the
>>>> if (X)
>>>> __builtin_unreachable ()
>>>>
>>>> idiom and set the appropriate global ranges, but it has to leave those
>>>> with 2 ssa-names:
>>>>
>>>> if (a_1 != b_2)
>>>> __builtin_unreachable()
>>>>
>>>> until the second pass of VRP or we lose the relationship between a_1 and
>>>> b_2. That triggers some failures. Specifically a vectorizor fail
>>>> because it cant be sure that the start and end point are not the same
>>>> without the condition in the IL. Trying to store global relations over
>>>> multiple passes would be problematic at this stage of development, so I
>>>> don't see a problem with leaving it that way.
>>> Hmm, I don't remember VRP1 doing anything special with the above though?
>>> Did it somehow propagate the (un!)conditional equivalence?
>> So as I looked at builtin_unreachable(), it was very adhoc. That one of the roots of that artificial testcase in the PR I opened. Cascading calls were not being handled in a consistent way. VRP1 removed some, dom removed some.. they just kind of disappeared at some point, but not consistently. The PR that Uli opened that Aldy fixed, I could make fail again with minor adjustments to the conditions. So I worked on a consistent approach.
>>
>> My guess is the old range stored globally for that case for a_1 was probably ~[b_2, b_2] meaning it was carried in the range. Until we have an overall global relation tracker, we can't represent that across passes.
> The global ranges were never symbolic, this was at most used during VRP itself.
>
>
Ah. Just took a closer look at what use to happen.
legacy vrp1 never removed the unreachable call, it hung around until the
threadfull2 ran just before vrp2. The testcase was an artificial
vectorizing test with an infinite loop and unreachable in the final
block. Just part of the inconsistent removal :-P:
Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: RFC - VRP1 default mode
2022-10-28 13:43 ` Andrew MacLeod
2022-10-28 13:46 ` Richard Biener
@ 2022-10-28 17:45 ` Eric Botcazou
1 sibling, 0 replies; 9+ messages in thread
From: Eric Botcazou @ 2022-10-28 17:45 UTC (permalink / raw)
To: Andrew MacLeod; +Cc: Richard Biener, gcc-patches, Jakub Jelinek, gcc-patches
> I get a clean testsuite run configured and bootstrapped with
>
> --enable-languages=c,c++,go,fortran,ada,obj-c++,jit --enable-host-shared
>
> Is there a PR or specific tests in either fortran or ada for those
> improvements? ie, something specific I should check for? Part of rangers
> point is to be able to do symbolic relationships without storing the
> symbolic in the range, just picking it up from the IL as needed.
The motivating Ada example for symbolic ranges was gnat.dg/opt40.adb.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-10-28 17:45 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-26 14:24 RFC - VRP1 default mode Andrew MacLeod
2022-10-26 14:59 ` Jeff Law
2022-10-28 7:17 ` Richard Biener
2022-10-28 13:43 ` Andrew MacLeod
2022-10-28 13:46 ` Richard Biener
2022-10-28 13:59 ` Andrew MacLeod
2022-10-28 14:14 ` Richard Biener
2022-10-28 14:33 ` Andrew MacLeod
2022-10-28 17:45 ` Eric Botcazou
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).