On 2/21/25 07:41, Paolo Bonzini wrote:
Ok, this one definitely caught my eye. :)
On 2/17/25 00:09, Richard Henderson wrote:
+ tcg_gen_add_i32(t0, a, b);
+ tcg_gen_setcond_i32(TCG_COND_LTU, t1, t0, a);
Compare against b instead? If there's an immediate (which could even be
zero) it is there.
Sure.
+ tcg_gen_add_i32(r, t0, ci);
+ tcg_gen_setcond_i32(TCG_COND_LTU, t0, r, t0);
+ tcg_gen_or_i32(co, t0, t1);
setcond + or can become a movcond:
For the set of hosts that require this support (mips, riscv, loongarch), this isn't really
an improvement -- we'll end up emitting a setcond LTU anyway and using that to feed
movcond NE instruction.
+ if (tcg_op_supported(INDEX_op_addci, TCG_TYPE_I32, 0)) {
+ TCGv_i32 discard = tcg_temp_ebb_new_i32();
+ TCGv_i32 zero = tcg_constant_i32(0);
+ TCGv_i32 mone = tcg_constant_i32(-1);
+
+ tcg_gen_op3_i32(INDEX_op_addco, discard, TCGV_LOW(ci), mone);
+ tcg_gen_op3_i32(INDEX_op_addcio, discard, TCGV_HIGH(ci), mone);
This addcio is unnecessary/incorrect. I think you assume that TCGV_HIGH(ci) =
0,
since that's what you set it below, and then this you can remove it.
I am *not* assuming TCGV_HIGH(ci) == 0.
I briefly thought about ignoring the high part, because "of course" carry-in is just the
low bit. However, the code emitted for a 64-bit host will not ignore the upper 32 bits,
so I thought it better to not intentionally change semantics.
+ } else {
+ TCGv_i32 t0 = tcg_temp_ebb_new_i32();
+ TCGv_i32 c0 = tcg_temp_ebb_new_i32();
+ TCGv_i32 c1 = tcg_temp_ebb_new_i32();
+
+ tcg_gen_or_i32(c1, TCGV_LOW(ci), TCGV_HIGH(ci));
+ tcg_gen_setcondi_i32(TCG_COND_NE, c1, c1, 0);
Likewise, this shouldn't be needed and you can just add TCGV_LOW(ci)
below.
Again, this is about keeping the semantics the same as x86_64, where a non-zero ci is
treated as a single carry-in bit.
r~
