On 10/2/21 12:39, Peter Maydell wrote:
On Thu, 30 Sept 2021 at 06:44, David Gibson <da...@gibson.dropbear.id.au> wrote:From: Cédric Le Goater <c...@kaod.org> The current way the mask is built can overflow with a 64-bit decrementer. Use sextract64() to extract the signed values and remove the logic to handle negative values which has become useless. Cc: Luis Fernando Fujita Pires <luis.pi...@eldorado.org.br> Fixes: a8dafa525181 ("target/ppc: Implement large decrementer support for TCG") Signed-off-by: Cédric Le Goater <c...@kaod.org> Message-Id: <20210920061203.989563-5-...@kaod.org> Reviewed-by: Luis Pires <luis.pi...@eldorado.org.br> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>Hi; Coverity complains about dead code here (CID 1464061):* On MSB edge based DEC implementations the MSB going from 0 -> 1 triggers * an edge interrupt, so raise it here too. */ - if ((value < 3) || - ((tb_env->flags & PPC_DECR_UNDERFLOW_LEVEL) && negative) || - ((tb_env->flags & PPC_DECR_UNDERFLOW_TRIGGERED) && negative - && !(decr & (1ULL << (nr_bits - 1))))) { + if ((signed_value < 3) || + ((tb_env->flags & PPC_DECR_UNDERFLOW_LEVEL) && signed_value < 0) || + ((tb_env->flags & PPC_DECR_UNDERFLOW_TRIGGERED) && signed_value < 0 + && signed_decr >= 0)) { (*raise_excp)(cpu); return; }If signed_value < 3 then the first clause of the || evaluates as true, and so we will definitely take the if() clause. So if we're evaluating the second operand to the || then we know that signed_value > 3, which means that 'signed_value < 0' is always false and in turn that neither of the other two '||' options can be true. The whole expression is equivalent to just "if (signed_value < 3)". What was intended here? If this was supposed to be a no-behaviour-change commit (apart from fixing the 64-bit case) then the condition should have stayed as "(value < 3)", I think.
Yes. That was the intention. I will take a closer look. Thanks, C.
