From: Marc Glisse <marc.glisse@inria.fr>
To: Richard Biener <richard.guenther@gmail.com>
Cc: Aldy Hernandez <aldyh@redhat.com>,
Martin Sebor <msebor@redhat.com>,
GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: Simplify more EXACT_DIV_EXPR comparisons
Date: Wed, 29 May 2019 21:33:00 -0000 [thread overview]
Message-ID: <alpine.DEB.2.21.1905292317310.16738@stedding.saclay.inria.fr> (raw)
In-Reply-To: <CAFiYyc3an7zLcVxFgrJN-KPb4DoAg=Tz2Ow2_X51y0hJpT50Zg@mail.gmail.com>
On Mon, 20 May 2019, Richard Biener wrote:
> On Mon, May 20, 2019 at 10:16 AM Marc Glisse <marc.glisse@inria.fr> wrote:
>>
>> On Mon, 20 May 2019, Richard Biener wrote:
>>
>>> On Sun, May 19, 2019 at 6:16 PM Marc Glisse <marc.glisse@inria.fr> wrote:
>>>>
>>>> Hello,
>>>>
>>>> 2 pieces:
>>>>
>>>> - the first one handles the case where the denominator is negative. It
>>>> doesn't happen often with exact_div, so I don't handle it everywhere, but
>>>> this one looked trivial
>>>>
>>>> - handle the case where a pointer difference is cast to an unsigned type
>>>> before being compared to a constant (I hit this in std::vector). With some
>>>> range info we could probably handle some non-constant cases as well...
>>>>
>>>> The second piece breaks Walloca-13.c (-Walloca-larger-than=100 -O2)
>>>>
>>>> void f (void*);
>>>> void g (int *p, int *q)
>>>> {
>>>> __SIZE_TYPE__ n = (__SIZE_TYPE__)(p - q);
>>>> if (n < 100)
>>>> f (__builtin_alloca (n));
>>>> }
>>>>
>>>> At the time of walloca2, we have
>>>>
>>>> _1 = p_5(D) - q_6(D);
>>>> # RANGE [-2305843009213693952, 2305843009213693951]
>>>> _2 = _1 /[ex] 4;
>>>> # RANGE ~[2305843009213693952, 16140901064495857663]
>>>> n_7 = (long unsigned intD.10) _2;
>>>> _11 = (long unsigned intD.10) _1;
>>>> if (_11 <= 396)
>>>> [...]
>>>> _3 = allocaD.1059 (n_7);
>>>>
>>>> and warn.
>>>
>>> That's indeed to complicated relation of _11 to n_7 for
>>> VRP predicate discovery.
>>>
>>>> However, DOM3 later produces
>>>>
>>>> _1 = p_5(D) - q_6(D);
>>>> _11 = (long unsigned intD.10) _1;
>>>> if (_11 <= 396)
>>>
>>> while _11 vs. _1 works fine.
>>>
>>>> [...]
>>>> # RANGE [0, 99] NONZERO 127
>>>> _2 = _1 /[ex] 4;
>>>> # RANGE [0, 99] NONZERO 127
>>>> n_7 = (long unsigned intD.10) _2;
>>>> _3 = allocaD.1059 (n_7);
>>>>
>>>> so I am tempted to say that the walloca2 pass is too early, xfail the
>>>> testcase and file an issue...
>>>
>>> Hmm, there's a DOM pass before walloca2 already and moving
>>> walloca2 after loop opts doesn't look like the best thing to do?
>>> I suppose it's not DOM but sinking that does the important transform
>>> here? That is,
>>>
>>> Index: gcc/passes.def
>>> ===================================================================
>>> --- gcc/passes.def (revision 271395)
>>> +++ gcc/passes.def (working copy)
>>> @@ -241,9 +241,9 @@ along with GCC; see the file COPYING3.
>>> NEXT_PASS (pass_optimize_bswap);
>>> NEXT_PASS (pass_laddress);
>>> NEXT_PASS (pass_lim);
>>> - NEXT_PASS (pass_walloca, false);
>>> NEXT_PASS (pass_pre);
>>> NEXT_PASS (pass_sink_code);
>>> + NEXT_PASS (pass_walloca, false);
>>> NEXT_PASS (pass_sancov);
>>> NEXT_PASS (pass_asan);
>>> NEXT_PASS (pass_tsan);
>>>
>>> fixes it?
>>
>> I will check, but I don't think walloca uses any kind of on-demand VRP, so
>> we still need some pass to update the ranges after sinking, which doesn't
>> seem to happen until the next DOM pass.
>
> Oh, ok... Aldy, why's this a separate pass anyways? I think similar
> other warnigns are emitted from RTL expansion? So maybe we can
> indeed move the pass towards warn_restrict or late_warn_uninit.
I tried moving it after 'sink' and that didn't help. Moving it next to
warn_restrict works for this test but breaks 2 others that currently
"work" by accident (+ one where the message changes between "unbounded"
and "too large", it isn't clear what the difference is between those
messages).
My suggestion, in addition to the original patch, is
* gcc.dg/Walloca-13.c: Xfail.
--- Walloca-13.c (revision 271742)
+++ Walloca-13.c (working copy)
@@ -1,12 +1,12 @@
/* { dg-do compile } */
/* { dg-require-effective-target alloca } */
/* { dg-options "-Walloca-larger-than=100 -O2" } */
void f (void*);
void g (int *p, int *q)
{
__SIZE_TYPE__ n = (__SIZE_TYPE__)(p - q);
if (n < 100)
- f (__builtin_alloca (n));
+ f (__builtin_alloca (n)); // { dg-bogus "may be too large due to conversion" "" { xfail { *-*-* } } }
}
Is that ok?
> I also see that the Og pass pipeline misses the second walloca pass
> completely (and also the warn_restrict pass).
>
> Given code sinkings obvious effects on SSA value-range representation
> it may make sense to add another instance of that pass earlier.
--
Marc Glisse
next prev parent reply other threads:[~2019-05-29 21:28 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-19 16:16 Marc Glisse
2019-05-20 8:04 ` Richard Biener
2019-05-20 8:16 ` Marc Glisse
2019-05-20 9:16 ` Richard Biener
2019-05-20 17:28 ` Jeff Law
2019-05-20 17:38 ` Marc Glisse
2019-05-20 17:41 ` Jeff Law
2019-05-21 2:13 ` Martin Sebor
2019-05-21 9:53 ` Richard Biener
2019-05-27 13:39 ` Aldy Hernandez
2019-05-27 13:58 ` Richard Biener
2019-05-27 14:44 ` Aldy Hernandez
2019-05-28 15:43 ` Martin Sebor
2019-05-29 7:47 ` Richard Biener
2019-05-29 16:31 ` Jeff Law
2019-05-29 21:33 ` Marc Glisse [this message]
2019-05-30 16:38 ` Jeff Law
2019-05-31 9:03 ` Aldy Hernandez
2019-05-31 15:51 ` Marc Glisse
2019-05-31 16:00 ` Aldy Hernandez
2019-05-31 9:00 ` Richard Biener
2019-05-20 17:32 ` Jeff Law
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.2.21.1905292317310.16738@stedding.saclay.inria.fr \
--to=marc.glisse@inria.fr \
--cc=aldyh@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=msebor@redhat.com \
--cc=richard.guenther@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).