On Mon, Feb 13, 2017 at 04:53:19PM -0700, Jeff Law wrote:
> > dirtype is one of the standard {un,}signed {char,short,int,long,long long}
> > types, all of them have 0 in their ranges.
> > For VR_RANGE we almost always set res.knownrange to true:
> > /* Set KNOWNRANGE if the argument is in a known subrange
> > of the directive's type (KNOWNRANGE may be reset below). */
> > res.knownrange
> > = (!tree_int_cst_equal (TYPE_MIN_VALUE (dirtype), argmin)
> > || !tree_int_cst_equal (TYPE_MAX_VALUE (dirtype), argmax));
> > (the exception is in case that range clearly has to include zero),
> > and reset it only if adjust_range_for_overflow returned true, which means
> > it also set the range to TYPE_M{IN,AX}_VALUE (dirtype) and again
> > includes zero.
> > So IMNSHO likely_adjust in what you've committed is always true
> > when you use it and thus just a useless computation and something to make
> > the code harder to understand.
> If KNOWNRANGE is false, then LIKELY_ADJUST will be true. But I don't see
> how we can determine anything for LIKELY_ADJUST if KNOWNRANGE is true.
We can't, but that doesn't matter, we only use it if KNOWNRANGE is false.
The only user of LIKELY_ADJUST is:
if (res.knownrange)
res.range.likely = res.range.max;
else
{
// -- Here we know res.knownrage is false
res.range.likely = res.range.min;
if (likely_adjust && maybebase && base != 10)
// -- and here is the only user of likely_adjust
{
if (res.range.min == 1)
res.range.likely += base == 8 ? 1 : 2;
else if (res.range.min == 2
&& base == 16
&& (dir.width[0] == 2 || dir.prec[0] == 2))
++res.range.likely;
}
}
> > Even if you don't trust this, with the ranges in argmin/argmax, it is
> > IMHO undesirable to set it differently at the different code paths,
> > if you want to check whether the final range includes zero and at least
> > one another value, just do
> > - if (likely_adjust && maybebase && base != 10)
> > + if ((tree_int_cst_sgn (argmin) < 0 || tree_int_cst_sgn (argmax) > 0)
> > && maybebase && base != 10)
> > Though, it is useless both for the above reason and for the reason that you
> > actually do something only:
> I'm not convinced it's useless, but it does seem advisable to bring test
> down to where it's actually used and to bse it strictly on argmin/argmax.
> Can you test a patch which does that?
That would then be (the only difference compared to the previous patch is
the last hunk) following. I can surely test that, I'm still convinced it
would work equally if that
(tree_int_cst_sgn (argmin) < 0 || tree_int_cst_sgn (argmax) > 0)
is just gcc_checking_assert.
2017-02-14 Jakub Jelinek <ja...@redhat.com>
PR tree-optimization/79327
* gimple-ssa-sprintf.c (format_integer): Remove likely_adjust
variable, its initialization and use.
--- gcc/gimple-ssa-sprintf.c.jj 2017-02-04 08:43:12.000000000 +0100
+++ gcc/gimple-ssa-sprintf.c 2017-02-04 08:45:33.173709580 +0100
@@ -1232,10 +1232,6 @@ format_integer (const directive &dir, tr
of the format string by returning [-1, -1]. */
return fmtresult ();
- /* True if the LIKELY counter should be adjusted upward from the MIN
- counter to account for arguments with unknown values. */
- bool likely_adjust = false;
-
fmtresult res;
/* Using either the range the non-constant argument is in, or its
@@ -1265,14 +1261,6 @@ format_integer (const directive &dir, tr
res.argmin = argmin;
res.argmax = argmax;
-
- /* Set the adjustment for an argument whose range includes
- zero since that doesn't include the octal or hexadecimal
- base prefix. */
- wide_int wzero = wi::zero (wi::get_precision (min));
- if (wi::le_p (min, wzero, SIGNED)
- && !wi::neg_p (max))
- likely_adjust = true;
}
else if (range_type == VR_ANTI_RANGE)
{
@@ -1307,11 +1295,6 @@ format_integer (const directive &dir, tr
if (!argmin)
{
- /* Set the adjustment for an argument whose range includes
- zero since that doesn't include the octal or hexadecimal
- base prefix. */
- likely_adjust = true;
-
if (TREE_CODE (argtype) == POINTER_TYPE)
{
argmin = build_int_cst (pointer_sized_int_node, 0);
@@ -1371,7 +1354,8 @@ format_integer (const directive &dir, tr
else
{
res.range.likely = res.range.min;
- if (likely_adjust && maybebase && base != 10)
+ if (maybebase && base != 10
+ && (tree_int_cst_sgn (argmin) < 0 || tree_int_cst_sgn (argmax) > 0))
{
if (res.range.min == 1)
res.range.likely += base == 8 ? 1 : 2;
Jakub
