| Message ID | 20200117144454.22045-1-msc@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] Fix maybe-uninitialized error on powerpc | expand |
On Fri, 17 Jan 2020, Matheus Castanho wrote: > + > + /* t == 0 will never happen at this point, since 1/t can only be 0 if t is > + infinity, but before the division t == mantissa of x (exponent is 0). We > + can instruct the compiler to ignore this case. */ > + if (t == 0) > + __builtin_unreachable(); This needs a space before '()'. OK with that change, but Siddhesh will need to confirm if it's OK to push at this development stage, and you'll need to say if you need someone to push it for you.
On 18/01/20 3:34 am, Joseph Myers wrote: > On Fri, 17 Jan 2020, Matheus Castanho wrote: > >> + >> + /* t == 0 will never happen at this point, since 1/t can only be 0 if t is >> + infinity, but before the division t == mantissa of x (exponent is 0). We >> + can instruct the compiler to ignore this case. */ >> + if (t == 0) >> + __builtin_unreachable(); > > This needs a space before '()'. > > OK with that change, but Siddhesh will need to confirm if it's OK to push > at this development stage, and you'll need to say if you need someone to > push it for you. > This is fine. Thanks, Siddhesh
Siddhesh Poyarekar <siddhesh@sourceware.org> writes: > On 18/01/20 3:34 am, Joseph Myers wrote: >> On Fri, 17 Jan 2020, Matheus Castanho wrote: >> >>> + >>> + /* t == 0 will never happen at this point, since 1/t can only be 0 if t is >>> + infinity, but before the division t == mantissa of x (exponent is 0). We >>> + can instruct the compiler to ignore this case. */ >>> + if (t == 0) >>> + __builtin_unreachable(); >> >> This needs a space before '()'. Fixed. >> OK with that change, but Siddhesh will need to confirm if it's OK to push >> at this development stage, and you'll need to say if you need someone to >> push it for you. > > This is fine. Pushed as 9f8b135f76ac7943d1e108b7f6e816f526b2208c. Thanks!
On 1/17/20 11:24 PM, Tulio Magno Quites Machado Filho wrote: > Siddhesh Poyarekar <siddhesh@sourceware.org> writes: > >> On 18/01/20 3:34 am, Joseph Myers wrote: >>> On Fri, 17 Jan 2020, Matheus Castanho wrote: >>> >>>> + >>>> + /* t == 0 will never happen at this point, since 1/t can only be 0 if t is >>>> + infinity, but before the division t == mantissa of x (exponent is 0). We >>>> + can instruct the compiler to ignore this case. */ >>>> + if (t == 0) >>>> + __builtin_unreachable(); >>> >>> This needs a space before '()'. > > Fixed. > >>> OK with that change, but Siddhesh will need to confirm if it's OK to push >>> at this development stage, and you'll need to say if you need someone to >>> push it for you. >> >> This is fine. > > Pushed as 9f8b135f76ac7943d1e108b7f6e816f526b2208c. > > Thanks! > Thanks!
diff --git a/sysdeps/ieee754/dbl-64/mpa.c b/sysdeps/ieee754/dbl-64/mpa.c index 3bb8bff90d..0bcba09e24 100644 --- a/sysdeps/ieee754/dbl-64/mpa.c +++ b/sysdeps/ieee754/dbl-64/mpa.c @@ -871,6 +871,13 @@ __inv (const mp_no *x, mp_no *y, int p) z.e = 0; __mp_dbl (&z, &t, p); t = 1 / t; + + /* t == 0 will never happen at this point, since 1/t can only be 0 if t is + infinity, but before the division t == mantissa of x (exponent is 0). We + can instruct the compiler to ignore this case. */ + if (t == 0) + __builtin_unreachable(); + __dbl_mp (t, y, p); EY -= EX;
Changes for v2: - Use __builtin_unreachable so compiler ignores false-positive - Add comment with proper explanation ----8<---- The build has been failing on powerpc64le-linux-gnu with GCC 10 due to a maybe-uninitialized error: ../sysdeps/ieee754/dbl-64/mpa.c:875:6: error: ‘w.e’ may be used uninitialized in this function [-Werror=maybe-uninitialized] 875 | EY -= EX; | ^~ The warning is thrown because when __inv is called by __dvd *y is not initialized and if t == 0 before calling __dbl_mp, EY will stay uninitialized, as the function does not touch it in this case. However, since t will be set to 1/t before calling __dbl_mp, t == 0 will never happen, so we can instruct the compiler to ignore this case, which suppresses the warning. Tested on powerpc64le. --- sysdeps/ieee754/dbl-64/mpa.c | 7 +++++++ 1 file changed, 7 insertions(+)