Re: [PULL v2 28/60] tcg/optimize: Split out fold_extract2

[Richard Henderson]

On 11/9/21 5:52 PM, Peter Maydell wrote:
> On Fri, 29 Oct 2021 at 05:59, Richard Henderson
> <richard.henderson@linaro.org> wrote:
> > Reviewed-by: Luis Pires <luis.pires@eldorado.org.br>
> > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >   tcg/optimize.c | 39 ++++++++++++++++++++++-----------------
> >   1 file changed, 22 insertions(+), 17 deletions(-)
> >
> > diff --git a/tcg/optimize.c b/tcg/optimize.c
> > index 110b3d1cc2..faedbdbfb8 100644
> > --- a/tcg/optimize.c
> > +++ b/tcg/optimize.c
> > @@ -888,6 +888,25 @@ static bool fold_eqv(OptContext *ctx, TCGOp *op)
> >       return fold_const2(ctx, op);
> >   }
>
> Hi; Coverity warns about a shift in here (CID 1465220):
>
> > +static bool fold_extract2(OptContext *ctx, TCGOp *op)
> > +{
> > +    if (arg_is_const(op->args[1]) && arg_is_const(op->args[2])) {
> > +        uint64_t v1 = arg_info(op->args[1])->val;
> > +        uint64_t v2 = arg_info(op->args[2])->val;
> > +        int shr = op->args[3];
> > +
> > +        if (op->opc == INDEX_op_extract2_i64) {
> > +            v1 >>= shr;
> > +            v2 <<= 64 - shr;
> > +        } else {
> > +            v1 = (uint32_t)v1 >> shr;
> > +            v2 = (int32_t)v2 << (32 - shr);
>
> Here we do the shift at 32-bits and then assign it into a 64-bit
> variable, which triggers Coverity's usual OVERFLOW_BEFORE_WIDEN
> heuristic. Is the 32-bitness intentional?

Yep.  I'll add another cast.


r~