On 23 August 2013 19:09, Richard Henderson <r...@twiddle.net> wrote: > On 08/23/2013 09:12 AM, Peter Maydell wrote: >> - offset = (((int32_t)insn << 8) >> 8); >> - val += (offset << 2) + 4; >> + offset = sextract32(insn << 2, 0, 26); >> + val += offset + 4; > > I read this incorrectly at first, considering the shift of insn, and > I wonder if it's really the best way to write this because of that. > > What about just changing the one line to sextract(insn, 0, 24)? > > The second line by itself ought not trigger a warning from clang, > because the << 2 never changes the sign bit. If it still does, > perhaps just multiply by 4 instead...
No, left shift of a negative value is undefined: "If E1 has a signed type and nonnegative value, and E1 × 2E2 is representable in the result type, then that is the resulting value; otherwise, the behavior is undefined." Also the ARM ARM pseudocode defines this operation as "first append two zero bits and then sign extend" so I prefer it if we actually implement it that way round. > It's a stupid warning. When was the last ones-compliment machine built? The stupidity is that the C standard hasn't mandated 2s-complement. -- PMM
