On 06/09/2011 03:45 AM, Kirill Batuzov wrote:
> +static int op_to_mov(int op)
> +{
> + if (op_bits(op) == 32) {
> + return INDEX_op_mov_i32;
> + }
> +#if TCG_TARGET_REG_BITS == 64
> + if (op_bits(op) == 64) {
> + return INDEX_op_mov_i64;
> + }
> +#endif
> + tcg_abort();
> +}
Again, switch not two calls.
> +static TCGArg do_constant_folding(int op, TCGArg x, TCGArg y)
> +{
> + TCGArg res = do_constant_folding_2(op, x, y);
> +#if TCG_TARGET_REG_BITS == 64
> + if (op_bits(op) == 32) {
> + res &= 0xffffffff;
Strictly speaking, this isn't required. The top bits of any
constant are considered garbage to the code generators. C.f.
the code in tcg_out_movi for any 64-bit host.
That said, only x86_64 and s390 get this right for the constraints.
x86_64 by being able to use 'i' to accept all constants for 32-bit
operations, and s390 by using 'W' as a modifier to force 32-bit
comparison in tcg_target_const_match.
So it's probably best to keep this for now.
> + /* Simplify expression if possible. */
> + switch (op) {
> + case INDEX_op_add_i32:
> +#if TCG_TARGET_REG_BITS == 64
> + case INDEX_op_add_i64:
> +#endif
> + if (temps[args[1]].state == TCG_TEMP_CONST) {
> + /* 0 + x == x + 0 */
> + tmp = args[1];
> + args[1] = args[2];
> + args[2] = tmp;
> + }
Probably best to break this out into another switch so that
you can handle all of the commutative operations.
> + /* Fallthrough */
> + case INDEX_op_sub_i32:
> +#if TCG_TARGET_REG_BITS == 64
> + case INDEX_op_sub_i64:
> +#endif
> + if (temps[args[1]].state == TCG_TEMP_CONST) {
> + /* Proceed with possible constant folding. */
> + break;
> + }
> + if (temps[args[2]].state == TCG_TEMP_CONST
> + && temps[args[2]].val == 0) {
> + if ((temps[args[0]].state == TCG_TEMP_COPY
> + && temps[args[0]].val == args[1])
> + || args[0] == args[1]) {
> + args += 3;
> + gen_opc_buf[op_index] = INDEX_op_nop;
> + } else {
> + reset_temp(temps, args[0], nb_temps, nb_globals);
> + if (args[1] >= s->nb_globals) {
> + temps[args[0]].state = TCG_TEMP_COPY;
> + temps[args[0]].val = args[1];
> + temps[args[1]].num_copies++;
> + }
> + gen_opc_buf[op_index] = op_to_mov(op);
> + gen_args[0] = args[0];
> + gen_args[1] = args[1];
> + gen_args += 2;
> + args += 3;
> + }
> + continue;
> + }
> + break;
> + case INDEX_op_mul_i32:
> +#if TCG_TARGET_REG_BITS == 64
> + case INDEX_op_mul_i64:
> +#endif
> + if ((temps[args[1]].state == TCG_TEMP_CONST
> + && temps[args[1]].val == 0)
> + || (temps[args[2]].state == TCG_TEMP_CONST
> + && temps[args[2]].val == 0)) {
> + reset_temp(temps, args[0], nb_temps, nb_globals);
> + temps[args[0]].state = TCG_TEMP_CONST;
> + temps[args[0]].val = 0;
> + assert(temps[args[0]].num_copies == 0);
> + gen_opc_buf[op_index] = op_to_movi(op);
> + gen_args[0] = args[0];
> + gen_args[1] = 0;
> + args += 3;
> + gen_args += 2;
> + continue;
> + }
This is *way* too much code duplication, particularly with the
same code sequences already existing for mov and movi and more
to come with the logicals.
r~
