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

[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.