Am 2015-02-20 15:36, schrieb Paolo Bonzini:
On 20/02/2015 15:18, Radim Krčmář 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];
^
Signed-off-by: Radim Krčmář <rkrc...@redhat.com>
---
hw/misc/milkymist-pfpu.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/hw/misc/milkymist-pfpu.c b/hw/misc/milkymist-pfpu.c
index 609f33f9cd14..133f5b8c5153 100644
--- a/hw/misc/milkymist-pfpu.c
+++ b/hw/misc/milkymist-pfpu.c
@@ -164,6 +164,13 @@ output_queue_advance(MilkymistPFPUState *s)
static int pfpu_decode_insn(MilkymistPFPUState *s)
{
uint32_t pc = s->regs[R_PC];
+
+ if (pc > MICROCODE_WORDS) {
+ error_report("milkymist_pfpu: too many instructions "
+ "executed in microcode. No VECTOUT?");
+ return 0;
+ }
+
I don't like this syntax, eg declaration, statements, declaration. Can
you just declare the variable first and then assign them? Also the error
message is then misleading. I'd prefer something like "milkymist_pfpu:
program counter out of bounds. No VECTOUT?"
uint32_t insn = s->microcode[pc];
uint32_t reg_a = (insn >> 18) & 0x7f;
uint32_t reg_b = (insn >> 11) & 0x7f;
@@ -348,7 +355,6 @@ static int pfpu_decode_insn(MilkymistPFPUState *s)
static void pfpu_start(MilkymistPFPUState *s)
{
int x, y;
- int i;
for (y = 0; y <= s->regs[R_VMESHLAST]; y++) {
for (x = 0; x <= s->regs[R_HMESHLAST]; x++) {
@@ -359,15 +365,7 @@ static void pfpu_start(MilkymistPFPUState *s)
s->gp_regs[GPR_Y] = y;
/* run microcode on this position */
- i = 0;
- while (pfpu_decode_insn(s)) {
- /* decode at most MICROCODE_WORDS instructions */
- if (i++ >= MICROCODE_WORDS) {
Isn't the fix just to say "++i" instead of "i++"?
In the first run, s->regs[R_PC] may have any value, therefore the "insn
= s->microcode[pc]" from above may access out of bounds.
-michael
