Re: [PATCH v2 21/22] tcg/tci: Split out constraint sets to tcg-target-con-set.h

[Richard Henderson]

On 1/19/21 6:09 AM, Peter Maydell wrote:
> On Fri, 15 Jan 2021 at 21:23, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> This requires finishing the conversion to tcg_target_op_def.
>> Remove quite a lot of ifdefs, since we can reference opcodes
>> even if they are not implemented.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> 
> This one's a lot more painful to review than the native targets :-(
> 
>> ---
> 
>> -/* TODO: documentation. */
>> -static const TCGTargetOpDef tcg_target_op_defs[] = {
>> -    { INDEX_op_exit_tb, { NULL } },
>> -    { INDEX_op_goto_tb, { NULL } },
>> -    { INDEX_op_br, { NULL } },
> 
> I don't see any cases in the new code for these ops,
> or for INDEX_op_mb which has {}. Is the function in fact
> never called for those ops ?

Correct.  Just before tcg_target_op_def() is called:

        if (nb_args == 0) {
            continue;
        }

>> -#if TCG_TARGET_HAS_div_i32
>> -    { INDEX_op_div_i32, { R, R, R } },
>> -    { INDEX_op_divu_i32, { R, R, R } },
>> -    { INDEX_op_rem_i32, { R, R, R } },
>> -    { INDEX_op_remu_i32, { R, R, R } },
>> -#elif TCG_TARGET_HAS_div2_i32
>> -    { INDEX_op_div2_i32, { R, R, "0", "1", R } },
>> -    { INDEX_op_divu2_i32, { R, R, "0", "1", R } },
>> -#endif
> 
> Why don't we need all the ifdeffery the old code has ? Is
> it because we know the ifdefs are always true (or always false) ?
> If so, can we do the "drop ifdefs" in a separate patch beforehand?
> I think that might help make the patch a bit easier to review.

Ok, I've split this up a bit.


r~