* [PATCH] Adjust gimple-ssa-sprintf.c for irange API.
@ 2020-08-04 11:21 Aldy Hernandez
2020-08-04 13:59 ` Martin Sebor
0 siblings, 1 reply; 4+ messages in thread
From: Aldy Hernandez @ 2020-08-04 11:21 UTC (permalink / raw)
To: gcc-patches; +Cc: law, msebor, Aldy Hernandez
This is a rather obvious patch, but I'd like a nod before committing.
Martin, I've removed your anti-range check, as it is subsumed by the
lower_bound/upper_bound code. However, you will have to adapt the code
for multi-ranges if desired. For example, you may want to loop through the
sub-ranges and do the right thing. Look at value-range.h and see the comments
for class irange. Those are the methods you should stick to.
i.e.
for (i=0; i < vr->num_pairs(); ++i)
stuff_with(vr->lower_bound(i), vr->upper_bound(i))
There should be no functional changes with this patch.
Aldy
gcc/ChangeLog:
* gimple-ssa-sprintf.c (get_int_range): Adjust for irange API.
(format_integer): Same.
(handle_printf_call): Same.
---
gcc/gimple-ssa-sprintf.c | 37 ++++++++++++++++---------------------
1 file changed, 16 insertions(+), 21 deletions(-)
diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 3d77459d811..70b031fe7b9 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -1070,7 +1070,7 @@ get_int_range (tree arg, HOST_WIDE_INT *pmin, HOST_WIDE_INT *pmax,
const value_range_equiv *vr
= CONST_CAST (class vr_values *, vr_values)->get_value_range (arg);
- if (range_int_cst_p (vr))
+ if (!vr->undefined_p () && !vr->varying_p () && !vr->symbolic_p ())
{
HOST_WIDE_INT type_min
= (TYPE_UNSIGNED (argtype)
@@ -1079,8 +1079,11 @@ get_int_range (tree arg, HOST_WIDE_INT *pmin, HOST_WIDE_INT *pmax,
HOST_WIDE_INT type_max = tree_to_uhwi (TYPE_MAX_VALUE (argtype));
- *pmin = TREE_INT_CST_LOW (vr->min ());
- *pmax = TREE_INT_CST_LOW (vr->max ());
+ tree type = TREE_TYPE (arg);
+ tree tmin = wide_int_to_tree (type, vr->lower_bound ());
+ tree tmax = wide_int_to_tree (type, vr->upper_bound ());
+ *pmin = TREE_INT_CST_LOW (tmin);
+ *pmax = TREE_INT_CST_LOW (tmax);
if (*pmin < *pmax)
{
@@ -1372,10 +1375,10 @@ format_integer (const directive &dir, tree arg, const vr_values *vr_values)
const value_range_equiv *vr
= CONST_CAST (class vr_values *, vr_values)->get_value_range (arg);
- if (range_int_cst_p (vr))
+ if (!vr->varying_p () && !vr->undefined_p () && !vr->symbolic_p ())
{
- argmin = vr->min ();
- argmax = vr->max ();
+ argmin = wide_int_to_tree (TREE_TYPE (arg), vr->lower_bound ());
+ argmax = wide_int_to_tree (TREE_TYPE (arg), vr->upper_bound ());
/* Set KNOWNRANGE if the argument is in a known subrange
of the directive's type and neither width nor precision
@@ -1388,11 +1391,7 @@ format_integer (const directive &dir, tree arg, const vr_values *vr_values)
res.argmin = argmin;
res.argmax = argmax;
}
- else if (vr->kind () == VR_ANTI_RANGE)
- {
- /* Handle anti-ranges if/when bug 71690 is resolved. */
- }
- else if (vr->varying_p () || vr->undefined_p ())
+ else
{
/* The argument here may be the result of promoting the actual
argument to int. Try to determine the type of the actual
@@ -4561,10 +4560,13 @@ handle_printf_call (gimple_stmt_iterator *gsi, const vr_values *vr_values)
const value_range_equiv *vr
= CONST_CAST (class vr_values *, vr_values)->get_value_range (size);
- if (range_int_cst_p (vr))
+ if (!vr->undefined_p () && !vr->symbolic_p ())
{
- unsigned HOST_WIDE_INT minsize = TREE_INT_CST_LOW (vr->min ());
- unsigned HOST_WIDE_INT maxsize = TREE_INT_CST_LOW (vr->max ());
+ tree type = TREE_TYPE (size);
+ tree tmin = wide_int_to_tree (type, vr->lower_bound ());
+ tree tmax = wide_int_to_tree (type, vr->upper_bound ());
+ unsigned HOST_WIDE_INT minsize = TREE_INT_CST_LOW (tmin);
+ unsigned HOST_WIDE_INT maxsize = TREE_INT_CST_LOW (tmax);
dstsize = warn_level < 2 ? maxsize : minsize;
if (minsize > target_int_max ())
@@ -4578,13 +4580,6 @@ handle_printf_call (gimple_stmt_iterator *gsi, const vr_values *vr_values)
if (maxsize > target_int_max ())
posunder4k = false;
}
- else if (vr->varying_p ())
- {
- /* POSIX requires snprintf to fail if DSTSIZE is greater
- than INT_MAX. Since SIZE's range is unknown, avoid
- folding. */
- posunder4k = false;
- }
/* The destination size is not constant. If the function is
bounded (e.g., snprintf) a lower bound of zero doesn't
--
2.26.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Adjust gimple-ssa-sprintf.c for irange API.
2020-08-04 11:21 [PATCH] Adjust gimple-ssa-sprintf.c for irange API Aldy Hernandez
@ 2020-08-04 13:59 ` Martin Sebor
2020-08-04 14:11 ` Aldy Hernandez
0 siblings, 1 reply; 4+ messages in thread
From: Martin Sebor @ 2020-08-04 13:59 UTC (permalink / raw)
To: Aldy Hernandez, gcc-patches; +Cc: msebor
On 8/4/20 5:21 AM, Aldy Hernandez via Gcc-patches wrote:
> This is a rather obvious patch, but I'd like a nod before committing.
>
> Martin, I've removed your anti-range check, as it is subsumed by the
> lower_bound/upper_bound code. However, you will have to adapt the code
> for multi-ranges if desired. For example, you may want to loop through the
> sub-ranges and do the right thing. Look at value-range.h and see the comments
> for class irange. Those are the methods you should stick to.
>
> i.e.
> for (i=0; i < vr->num_pairs(); ++i)
> stuff_with(vr->lower_bound(i), vr->upper_bound(i))
>
> There should be no functional changes with this patch.
I have no concern with this change but I appreciate the heads
up and the tip on how to add the multi-range support. Just
one suggestion: I'd prefer to keep the comment about the POSIX
requirement somewhere just as a reminder.
Thanks
Martin
>
> Aldy
>
> gcc/ChangeLog:
>
> * gimple-ssa-sprintf.c (get_int_range): Adjust for irange API.
> (format_integer): Same.
> (handle_printf_call): Same.
> ---
> gcc/gimple-ssa-sprintf.c | 37 ++++++++++++++++---------------------
> 1 file changed, 16 insertions(+), 21 deletions(-)
>
> diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
> index 3d77459d811..70b031fe7b9 100644
> --- a/gcc/gimple-ssa-sprintf.c
> +++ b/gcc/gimple-ssa-sprintf.c
> @@ -1070,7 +1070,7 @@ get_int_range (tree arg, HOST_WIDE_INT *pmin, HOST_WIDE_INT *pmax,
> const value_range_equiv *vr
> = CONST_CAST (class vr_values *, vr_values)->get_value_range (arg);
>
> - if (range_int_cst_p (vr))
> + if (!vr->undefined_p () && !vr->varying_p () && !vr->symbolic_p ())
> {
> HOST_WIDE_INT type_min
> = (TYPE_UNSIGNED (argtype)
> @@ -1079,8 +1079,11 @@ get_int_range (tree arg, HOST_WIDE_INT *pmin, HOST_WIDE_INT *pmax,
>
> HOST_WIDE_INT type_max = tree_to_uhwi (TYPE_MAX_VALUE (argtype));
>
> - *pmin = TREE_INT_CST_LOW (vr->min ());
> - *pmax = TREE_INT_CST_LOW (vr->max ());
> + tree type = TREE_TYPE (arg);
> + tree tmin = wide_int_to_tree (type, vr->lower_bound ());
> + tree tmax = wide_int_to_tree (type, vr->upper_bound ());
> + *pmin = TREE_INT_CST_LOW (tmin);
> + *pmax = TREE_INT_CST_LOW (tmax);
>
> if (*pmin < *pmax)
> {
> @@ -1372,10 +1375,10 @@ format_integer (const directive &dir, tree arg, const vr_values *vr_values)
> const value_range_equiv *vr
> = CONST_CAST (class vr_values *, vr_values)->get_value_range (arg);
>
> - if (range_int_cst_p (vr))
> + if (!vr->varying_p () && !vr->undefined_p () && !vr->symbolic_p ())
> {
> - argmin = vr->min ();
> - argmax = vr->max ();
> + argmin = wide_int_to_tree (TREE_TYPE (arg), vr->lower_bound ());
> + argmax = wide_int_to_tree (TREE_TYPE (arg), vr->upper_bound ());
>
> /* Set KNOWNRANGE if the argument is in a known subrange
> of the directive's type and neither width nor precision
> @@ -1388,11 +1391,7 @@ format_integer (const directive &dir, tree arg, const vr_values *vr_values)
> res.argmin = argmin;
> res.argmax = argmax;
> }
> - else if (vr->kind () == VR_ANTI_RANGE)
> - {
> - /* Handle anti-ranges if/when bug 71690 is resolved. */
> - }
> - else if (vr->varying_p () || vr->undefined_p ())
> + else
> {
> /* The argument here may be the result of promoting the actual
> argument to int. Try to determine the type of the actual
> @@ -4561,10 +4560,13 @@ handle_printf_call (gimple_stmt_iterator *gsi, const vr_values *vr_values)
> const value_range_equiv *vr
> = CONST_CAST (class vr_values *, vr_values)->get_value_range (size);
>
> - if (range_int_cst_p (vr))
> + if (!vr->undefined_p () && !vr->symbolic_p ())
> {
> - unsigned HOST_WIDE_INT minsize = TREE_INT_CST_LOW (vr->min ());
> - unsigned HOST_WIDE_INT maxsize = TREE_INT_CST_LOW (vr->max ());
> + tree type = TREE_TYPE (size);
> + tree tmin = wide_int_to_tree (type, vr->lower_bound ());
> + tree tmax = wide_int_to_tree (type, vr->upper_bound ());
> + unsigned HOST_WIDE_INT minsize = TREE_INT_CST_LOW (tmin);
> + unsigned HOST_WIDE_INT maxsize = TREE_INT_CST_LOW (tmax);
> dstsize = warn_level < 2 ? maxsize : minsize;
>
> if (minsize > target_int_max ())
> @@ -4578,13 +4580,6 @@ handle_printf_call (gimple_stmt_iterator *gsi, const vr_values *vr_values)
> if (maxsize > target_int_max ())
> posunder4k = false;
> }
> - else if (vr->varying_p ())
> - {
> - /* POSIX requires snprintf to fail if DSTSIZE is greater
> - than INT_MAX. Since SIZE's range is unknown, avoid
> - folding. */
> - posunder4k = false;
> - }
>
> /* The destination size is not constant. If the function is
> bounded (e.g., snprintf) a lower bound of zero doesn't
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Adjust gimple-ssa-sprintf.c for irange API.
2020-08-04 13:59 ` Martin Sebor
@ 2020-08-04 14:11 ` Aldy Hernandez
2020-08-04 14:39 ` Martin Sebor
0 siblings, 1 reply; 4+ messages in thread
From: Aldy Hernandez @ 2020-08-04 14:11 UTC (permalink / raw)
To: Martin Sebor; +Cc: gcc-patches, Martin Sebor
On Tue, Aug 4, 2020 at 3:59 PM Martin Sebor <msebor@gmail.com> wrote:
>
> On 8/4/20 5:21 AM, Aldy Hernandez via Gcc-patches wrote:
> > This is a rather obvious patch, but I'd like a nod before committing.
> >
> > Martin, I've removed your anti-range check, as it is subsumed by the
> > lower_bound/upper_bound code. However, you will have to adapt the code
> > for multi-ranges if desired. For example, you may want to loop through the
> > sub-ranges and do the right thing. Look at value-range.h and see the comments
> > for class irange. Those are the methods you should stick to.
> >
> > i.e.
> > for (i=0; i < vr->num_pairs(); ++i)
> > stuff_with(vr->lower_bound(i), vr->upper_bound(i))
> >
> > There should be no functional changes with this patch.
>
> I have no concern with this change but I appreciate the heads
> up and the tip on how to add the multi-range support. Just
> one suggestion: I'd prefer to keep the comment about the POSIX
> requirement somewhere just as a reminder.
The comment is still there, as you had a duplicate one further up:
else if (dstsize > target_int_max ())
{
warning_at (gimple_location (info.callstmt), info.warnopt (),
"specified bound %wu exceeds %<INT_MAX%>",
dstsize);
/* POSIX requires snprintf to fail if DSTSIZE is greater
than INT_MAX. Avoid folding in that case. */
posunder4k = false;
}
Are you ok with this, or would you rather me copy that comment somewhere else?
Thanks.
Aldy
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Adjust gimple-ssa-sprintf.c for irange API.
2020-08-04 14:11 ` Aldy Hernandez
@ 2020-08-04 14:39 ` Martin Sebor
0 siblings, 0 replies; 4+ messages in thread
From: Martin Sebor @ 2020-08-04 14:39 UTC (permalink / raw)
To: Aldy Hernandez; +Cc: gcc-patches, Martin Sebor
On 8/4/20 8:11 AM, Aldy Hernandez wrote:
> On Tue, Aug 4, 2020 at 3:59 PM Martin Sebor <msebor@gmail.com> wrote:
>>
>> On 8/4/20 5:21 AM, Aldy Hernandez via Gcc-patches wrote:
>>> This is a rather obvious patch, but I'd like a nod before committing.
>>>
>>> Martin, I've removed your anti-range check, as it is subsumed by the
>>> lower_bound/upper_bound code. However, you will have to adapt the code
>>> for multi-ranges if desired. For example, you may want to loop through the
>>> sub-ranges and do the right thing. Look at value-range.h and see the comments
>>> for class irange. Those are the methods you should stick to.
>>>
>>> i.e.
>>> for (i=0; i < vr->num_pairs(); ++i)
>>> stuff_with(vr->lower_bound(i), vr->upper_bound(i))
>>>
>>> There should be no functional changes with this patch.
>>
>> I have no concern with this change but I appreciate the heads
>> up and the tip on how to add the multi-range support. Just
>> one suggestion: I'd prefer to keep the comment about the POSIX
>> requirement somewhere just as a reminder.
>
> The comment is still there, as you had a duplicate one further up:
>
> else if (dstsize > target_int_max ())
> {
> warning_at (gimple_location (info.callstmt), info.warnopt (),
> "specified bound %wu exceeds %<INT_MAX%>",
> dstsize);
> /* POSIX requires snprintf to fail if DSTSIZE is greater
> than INT_MAX. Avoid folding in that case. */
> posunder4k = false;
> }
>
> Are you ok with this, or would you rather me copy that comment somewhere else?
I'm fine with it as is, I didn't see the other copy.
Thanks!
Martin
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-08-04 14:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-04 11:21 [PATCH] Adjust gimple-ssa-sprintf.c for irange API Aldy Hernandez
2020-08-04 13:59 ` Martin Sebor
2020-08-04 14:11 ` Aldy Hernandez
2020-08-04 14:39 ` Martin Sebor
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).