[Bug c/86380] New: Incorrect inequality in function chose_multiplier in file expmed.c in GCC.8.1.0 and earlier versions

[colinwb at yahoo dot co.uk]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86380

Bug ID: 86380
   Summary: Incorrect inequality in function chose_multiplier in
file expmed.c in GCC.8.1.0 and earlier versions
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c
  Assignee: unassigned at gcc dot gnu.org
  Reporter: colinwb at yahoo dot co.uk
  Target Milestone: ---

In this line near the end of function choose_multiplier in file expmed.c
return mhigh.to_uhwi () >= mask;
the ">=" should really be ">".
As it is when precision = n < HOST_BITS_PER_WIDE_INT and d = 2**(n-1) + 1,
so post_shift = 2*n - 1 - n = n - 1 and mhigh = 2**n - 1,
choose_multiplier wrongly returns that the upper bit should be set.

unsigned HOST_WIDE_INT
choose_multiplier (unsigned HOST_WIDE_INT d, int n, int precision,
   unsigned HOST_WIDE_INT *multiplier_ptr,
   int *post_shift_ptr, int *lgup_ptr)
...
  *post_shift_ptr = post_shift;
  *lgup_ptr = lgup;
  if (n < HOST_BITS_PER_WIDE_INT)
{
  unsigned HOST_WIDE_INT mask = (HOST_WIDE_INT_1U << n) - 1;
  *multiplier_ptr = mhigh.to_uhwi () & mask;
  return mhigh.to_uhwi () >= mask;
}
  else
{
  *multiplier_ptr = mhigh.to_uhwi ();
  return wi::extract_uhwi (mhigh, HOST_BITS_PER_WIDE_INT, 1);
}
}

[Bug middle-end/86380] incorrect comparison in function chose_multiplier

[colinwb at yahoo dot co.uk]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86380

--- Comment #2 from Colin Bartlett  ---
I suspected that when compiled programs didn't show the error, but:

1. Is that documented anywhere? It might be useful if that was documented also
in choose_multiplier?

2. Is that anyway any reason not to use the correct comparison, unless there is
a good reason to use the wrong comparison?

[Bug middle-end/86380] incorrect comparison in function chose_multiplier

[colinwb at yahoo dot co.uk]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86380

--- Comment #4 from Colin Bartlett  ---
>(In reply to Eric Botcazou from comment #3)
> > I suspected that when compiled programs didn't show the error, but:
> > 
> > 1. Is that documented anywhere? It might be useful if that was documented
> > also in choose_multiplier?
> > 
> > 2. Is that anyway any reason not to use the correct comparison, unless there
> > is a good reason to use the wrong comparison?
> 
> No and no.

In that case I suggest that:

(a) There's no need for any urgency at all for this as it won't affect
anything.

(b) But at an appropriate time in the future one of the following is done:

(b1) The comparison is changed to the correct comparison, maybe with a comment
that in previous versions ">=" was used which went wrong in the circumstances
set out above but choose_multiplier was never called with those arguments.

(b2) The comparison is left unchanged, but a comment is added that strictly
speaking we should use ">" but as choose_multiplier should never be called with
arguments for which ">" has a different effect to ">=" we can just use ">=".

My (strong) preference is for (b1).