qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 2/2] milkymist-pfpu: fix GCC 5.0.0 aggressive-lo


From: Michael Walle
Subject: Re: [Qemu-devel] [PATCH 2/2] milkymist-pfpu: fix GCC 5.0.0 aggressive-loop-optimizations warning
Date: Fri, 20 Feb 2015 17:10:26 +0100
User-agent: Roundcube Webmail/0.9.5

Am 2015-02-20 16:37, schrieb Radim Krčmář:
2015-02-20 23:40+0900, Peter Maydell:
On 20 February 2015 at 23:18, Radim Krčmář <address@hidden> wrote:
> man gcc:
>   Warn if in a loop with constant number of iterations the compiler
>   detects undefined behavior in some statement during one or more of
>   the iterations.
>
> Refactored the code a bit to avoid the GCC warning, in an objectionable
> way,
>   hw/misc/milkymist-pfpu.c: In function ‘pfpu_write’:
>   hw/misc/milkymist-pfpu.c:365:20: error: loop exit may only be reached after 
undefined behavior [-Werror=aggressive-loop-optimizations]
>                    if (i++ >= MICROCODE_WORDS) {
>                       ^
>   hw/misc/milkymist-pfpu.c:167:14: note: possible undefined statement is here
>        uint32_t insn = s->microcode[pc];
>                 ^

Why can't we just fix this by fixing the loop boundary
condition, which is the actual bug here? There should
be no need to move the check into the function (where it
does not belong).

It would work now, but GCC could get more intelligent with time and
realize that there still can be undefined behavior ...

(We try to stop before overflowing the s->microcode[]
array, but because 'i++' is a post increment and we do a >=
comparison, the last loop round will try to write to
s->microcode[MICROCODE_WORDS]. Easiest fix is to use
"++i" I suppose, though it might be better to just
separate the increment and the conditional instead.)

There is probably some actual hardware behaviour we're
failing to model correctly here, since it's a safe bet
the h/w doesn't print an error message in this situation.
However we should just fix the bogus array handling for now.

Ok, I will do just ++i for v2 and keep the other bug for later.

Ok, i'll post a patch later.

-michael



reply via email to

[Prev in Thread] Current Thread [Next in Thread]