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

[Michael Walle <michael@walle.cc>]

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ář <rkrcmar@redhat.com> 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