Re: [Qemu-devel] [PATCH 3/5] target-arm: convert shl and shr helpers to TCG

[Edgar E. Iglesias]

On Mon, Sep 17, 2012 at 03:36:46PM +0200, Laurent Desnogues wrote:
> On Mon, Sep 17, 2012 at 11:43 AM, Peter Maydell
> <address@hidden> wrote:
> > On 17 September 2012 10:30, Laurent Desnogues
> > <address@hidden> wrote:
> >> On Mon, Sep 17, 2012 at 1:08 AM, Aurelien Jarno <address@hidden> wrote:
> >>> +#define GEN_SHIFT(name)                                \
> >>> +static void gen_##name(TCGv dest, TCGv t0, TCGv t1)    \
> >>> +{                                                      \
> >>> +    TCGv tmp1, tmp2;                                   \
> >>> +    tmp1 = tcg_temp_new_i32();                         \
> >>> +    tcg_gen_andi_i32(tmp1, t1, 0xff);                  \
> >>> +    tmp2 = tcg_temp_new_i32();                         \
> >>> +    tcg_gen_setcondi_i32(TCG_COND_GE, tmp2, tmp1, 32); \
> >>> +    tcg_gen_andi_i32(tmp1, tmp1, 0x1f);                \
> >>
> >> I don't think the 'and 0x1f' is needed given that later you'll and
> >> with 0 if the shift amount is >= 32.
> >
> > The TCG shift operations are undefined behaviour (not merely
> > undefined result) if the shift is >= 32, so we must avoid
> > doing that even if we're going to throw away the answer.
> 
> That's odd that it doesn't just state that the result is undefined.
> I wonder what "undefined behavior" means.  I understand
> what undefined behavior (as opposed tu undefined result)
> means for divisions by 0, but not for a shift larger than data
> type width.
> 
> Anyway that makes my comment about removing the & 0x1f
> pointless.

Hi,

I'm a bit late with email but I don't think your comment is
irrelevant. It sounds like if we are optimizing for very odd
or nonexisting archs. Is there any modern arch that has
undefined behaviour (in reality and not only in theoretical
specs) these days?

Cheers,
E