* get_range_query vs NULL argument
@ 2023-02-15 19:50 Andrew Pinski
2023-02-15 22:30 ` Andrew MacLeod
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Pinski @ 2023-02-15 19:50 UTC (permalink / raw)
To: Aldy Hernandez, Andrew MacLeod, GCC Mailing List
Hi,
While fixing PR 108354, I came across that
ssa_name_has_boolean_range calls get_range_query with cfun as the
argument but sometimes while in IPA passes cfun is currently nullptr.
The question should we check the argument before calling
get_range_query or is it a valid thing to call it with a nullptr (and
have it return global_ranges in that case)?
I committed the patch (for PR 108354) with the workaround around the
issue via having the match.pd pattern which was calling
ssa_name_has_boolean_range only be invoked from the gimple
simplifiers.
Below is the patch which undones the workaround:
```
diff --git a/gcc/match.pd b/gcc/match.pd
index e7b700349a6..e352bd422f5 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -1732,7 +1732,6 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(if (!FIXED_POINT_TYPE_P (type))
(plus @0 (negate @1))))
-#if GIMPLE
/* 1 - a is a ^ 1 if a had a bool range. */
/* This is only enabled for gimple as sometimes
cfun is not set for the function which contains
@@ -1743,7 +1742,6 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(if (INTEGRAL_TYPE_P (type)
&& ssa_name_has_boolean_range (@1))
(bit_xor @1 @0)))
-#endif
/* Other simplifications of negation (c.f. fold_negate_expr_1). */
(simplify
```
Note I can only so far reproduce the call to
ssa_name_has_boolean_range that causes an issue while building Ada
tools (while bootstrapping) because the code that needs to hit this is
related to variable sized type accesses.
Thanks,
Andrew Pinski
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: get_range_query vs NULL argument
2023-02-15 19:50 get_range_query vs NULL argument Andrew Pinski
@ 2023-02-15 22:30 ` Andrew MacLeod
2023-02-16 7:57 ` Richard Biener
2023-02-17 17:13 ` Andrew Pinski
0 siblings, 2 replies; 5+ messages in thread
From: Andrew MacLeod @ 2023-02-15 22:30 UTC (permalink / raw)
To: Andrew Pinski, Aldy Hernandez, GCC Mailing List
On 2/15/23 14:50, Andrew Pinski wrote:
> Hi,
> While fixing PR 108354, I came across that
> ssa_name_has_boolean_range calls get_range_query with cfun as the
> argument but sometimes while in IPA passes cfun is currently nullptr.
> The question should we check the argument before calling
> get_range_query or is it a valid thing to call it with a nullptr (and
> have it return global_ranges in that case)?
That might be ok... personally I see nothing wrong with:
diff --git a/gcc/value-query.h b/gcc/value-query.h
index 63878968118..2d7bf8fcf33 100644
--- a/gcc/value-query.h
+++ b/gcc/value-query.h
@@ -140,7 +140,7 @@ get_global_range_query ()
ATTRIBUTE_RETURNS_NONNULL inline range_query *
get_range_query (const struct function *fun)
{
- return fun->x_range_query ? fun->x_range_query : &global_ranges;
+ return (fun && fun->x_range_query) ? fun->x_range_query : &global_ranges;
}
// Query the global range of NAME in function F. Default to cfun.
The client is probably going to do that anyway.
Aldy?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: get_range_query vs NULL argument
2023-02-15 22:30 ` Andrew MacLeod
@ 2023-02-16 7:57 ` Richard Biener
2023-02-16 8:10 ` Aldy Hernandez
2023-02-17 17:13 ` Andrew Pinski
1 sibling, 1 reply; 5+ messages in thread
From: Richard Biener @ 2023-02-16 7:57 UTC (permalink / raw)
To: Andrew MacLeod; +Cc: Andrew Pinski, Aldy Hernandez, GCC Mailing List
On Wed, Feb 15, 2023 at 11:31 PM Andrew MacLeod via Gcc <gcc@gcc.gnu.org> wrote:
>
>
> On 2/15/23 14:50, Andrew Pinski wrote:
> > Hi,
> > While fixing PR 108354, I came across that
> > ssa_name_has_boolean_range calls get_range_query with cfun as the
> > argument but sometimes while in IPA passes cfun is currently nullptr.
> > The question should we check the argument before calling
> > get_range_query or is it a valid thing to call it with a nullptr (and
> > have it return global_ranges in that case)?
>
> That might be ok... personally I see nothing wrong with:
>
> diff --git a/gcc/value-query.h b/gcc/value-query.h
> index 63878968118..2d7bf8fcf33 100644
> --- a/gcc/value-query.h
> +++ b/gcc/value-query.h
> @@ -140,7 +140,7 @@ get_global_range_query ()
> ATTRIBUTE_RETURNS_NONNULL inline range_query *
> get_range_query (const struct function *fun)
> {
> - return fun->x_range_query ? fun->x_range_query : &global_ranges;
> + return (fun && fun->x_range_query) ? fun->x_range_query : &global_ranges;
> }
>
> // Query the global range of NAME in function F. Default to cfun.
>
>
> The client is probably going to do that anyway.
But if there's no 'fun', what is 'global_ranges' initialized for? Or
is 'global_ranges'
usable in IPA context?
> Aldy?
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: get_range_query vs NULL argument
2023-02-16 7:57 ` Richard Biener
@ 2023-02-16 8:10 ` Aldy Hernandez
0 siblings, 0 replies; 5+ messages in thread
From: Aldy Hernandez @ 2023-02-16 8:10 UTC (permalink / raw)
To: Richard Biener, Andrew MacLeod; +Cc: Andrew Pinski, GCC Mailing List
On 2/16/23 08:57, Richard Biener wrote:
> On Wed, Feb 15, 2023 at 11:31 PM Andrew MacLeod via Gcc <gcc@gcc.gnu.org> wrote:
>>
>>
>> On 2/15/23 14:50, Andrew Pinski wrote:
>>> Hi,
>>> While fixing PR 108354, I came across that
>>> ssa_name_has_boolean_range calls get_range_query with cfun as the
>>> argument but sometimes while in IPA passes cfun is currently nullptr.
>>> The question should we check the argument before calling
>>> get_range_query or is it a valid thing to call it with a nullptr (and
>>> have it return global_ranges in that case)?
>>
>> That might be ok... personally I see nothing wrong with:
>>
>> diff --git a/gcc/value-query.h b/gcc/value-query.h
>> index 63878968118..2d7bf8fcf33 100644
>> --- a/gcc/value-query.h
>> +++ b/gcc/value-query.h
>> @@ -140,7 +140,7 @@ get_global_range_query ()
>> ATTRIBUTE_RETURNS_NONNULL inline range_query *
>> get_range_query (const struct function *fun)
>> {
>> - return fun->x_range_query ? fun->x_range_query : &global_ranges;
>> + return (fun && fun->x_range_query) ? fun->x_range_query : &global_ranges;
>> }
>>
>> // Query the global range of NAME in function F. Default to cfun.
>>
>>
>> The client is probably going to do that anyway.
>
> But if there's no 'fun', what is 'global_ranges' initialized for? Or
> is 'global_ranges'
> usable in IPA context?
If there is no fun, global_ranges will just return what get_range_info()
used to return (i.e. SSA_NAME_RANGE_INFO).
Aldy
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: get_range_query vs NULL argument
2023-02-15 22:30 ` Andrew MacLeod
2023-02-16 7:57 ` Richard Biener
@ 2023-02-17 17:13 ` Andrew Pinski
1 sibling, 0 replies; 5+ messages in thread
From: Andrew Pinski @ 2023-02-17 17:13 UTC (permalink / raw)
To: Andrew MacLeod; +Cc: Aldy Hernandez, GCC Mailing List
On Wed, Feb 15, 2023 at 2:30 PM Andrew MacLeod <amacleod@redhat.com> wrote:
>
>
> On 2/15/23 14:50, Andrew Pinski wrote:
> > Hi,
> > While fixing PR 108354, I came across that
> > ssa_name_has_boolean_range calls get_range_query with cfun as the
> > argument but sometimes while in IPA passes cfun is currently nullptr.
> > The question should we check the argument before calling
> > get_range_query or is it a valid thing to call it with a nullptr (and
> > have it return global_ranges in that case)?
>
> That might be ok... personally I see nothing wrong with:
>
> diff --git a/gcc/value-query.h b/gcc/value-query.h
> index 63878968118..2d7bf8fcf33 100644
> --- a/gcc/value-query.h
> +++ b/gcc/value-query.h
> @@ -140,7 +140,7 @@ get_global_range_query ()
> ATTRIBUTE_RETURNS_NONNULL inline range_query *
> get_range_query (const struct function *fun)
> {
> - return fun->x_range_query ? fun->x_range_query : &global_ranges;
> + return (fun && fun->x_range_query) ? fun->x_range_query : &global_ranges;
> }
>
> // Query the global range of NAME in function F. Default to cfun.
>
>
> The client is probably going to do that anyway.
Ok. I will submit a patch which does the above; I had that patch
already when I wrote the email and wanted to double check before
submitting it.
Thanks,
Andrew Pinski
>
> Aldy?
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-02-17 17:13 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-15 19:50 get_range_query vs NULL argument Andrew Pinski
2023-02-15 22:30 ` Andrew MacLeod
2023-02-16 7:57 ` Richard Biener
2023-02-16 8:10 ` Aldy Hernandez
2023-02-17 17:13 ` Andrew Pinski
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).