On 06/10/2010 03:22 AM, Aurelien Jarno wrote: >> - 0, GET_TCGV_I32(ret), 2, args); >> + (is_signed ? 0x2a : 0x00), GET_TCGV_I32(ret), 2, args); > > Wouldn't it be better to actually pass the whole flag to > tcg_gen_helper32(), so that we can in the future also support mixed > signedness in arguments? Also doing it here looks like a bit like a > magic constant.
I've fixed this. >> +#if defined(TCG_TARGET_EXTEND_ARGS) && TCG_TARGET_REG_BITS == 64 >> + for (i = 0; i < nargs; ++i) { >> + int is_64bit = sizemask & (1 << (i+1)*2); >> + int is_signed = sizemask & (2 << (i+1)*2); >> + if (!is_64bit) { >> + TCGv_i64 temp = tcg_temp_new_i64(); >> + TCGv_i64 orig = MAKE_TCGV_I64(args[i]); >> + if (is_signed) { >> + tcg_gen_ext32s_i64(temp, orig); >> + } else { >> + tcg_gen_ext32u_i64(temp, orig); >> + } >> + args[i] = GET_TCGV_I64(temp); >> + } >> + } >> +#endif /* TCG_TARGET_EXTEND_ARGS */ >> + > > This part allocates a lot of temp variables, that will probably generate > a lot of register spills during the code generation. > > As we do that for all arguments anyway, wouldn't it be possible to do > the extension in place? The value in the register is changed, but that > should not have any effect as it is ignored anyway in other > instructions. It is *not* possible to do the extension in-place. At least not without changing the format of the INDEX_op_call opcode. With the extension done during opcode generation, like this, ORIG has been marked TCG_TYPE_I32 and TEMP gets marked TCG_TYPE_I64. This matters when it comes time to copy the arguments into place. If we somehow extended ORIG in-place, we'd still use the wrong instruction to copy the value into the argument list -- either tcg_out_mov or tcg_out_st would get the wrong TYPE argument. If we try to do the extension later, e.g. while copying the value into the argument list, we'd need to have SIZEMASK available. To do that, we'd need to save SIZEMASK into INDEX_op_call's argument list somehow. That, I think, is a more invasive change. r~