Message ID | 000801d65db0$e45e7070$ad1b5150$@nextmovesoftware.com |
---|---|
State | New |
Headers | show |
Series | middle-end: Simplify (sign_extend:HI (truncate:QI (ashiftrt:HI X 8))) | expand |
Hi! On Sun, Jul 19, 2020 at 10:42:16AM +0100, Roger Sayle wrote: > This patch to simplify-rtx.c > simplifies (sign_extend:HI (truncate:QI (?shiftrt:HI x 8))) to just > (ashiftrt:HI x 8), as the inner shift already sets the high bits > appropriately. > The one oddity of the patch is that it tests for > LSHIFTRT as inner shift, as simplify/combine has already canonicalized > this to a logical shift, assuming that the distinction is unimportant > following the truncation. If simplify-rtx does that, that is a bug. But combine will do this, I think that is what you are seeing? Can you verify it already works if the ASHIFTRT is not changed to an LSHIFTRT? > + if (is_a <scalar_int_mode> (mode, &m_mode) > + && is_a <scalar_int_mode> (GET_MODE (op), &n_mode) > + && is_a <scalar_int_mode> (GET_MODE (old_shift), &o_mode) > + && GET_MODE_PRECISION (o_mode) - GET_MODE_PRECISION (n_mode) > + == INTVAL (XEXP (old_shift, 1))) > + { > + rtx new_shift = simplify_gen_binary (ASHIFTRT, > + GET_MODE (old_shift), > + XEXP (old_shift, 0), > + XEXP (old_shift, 1)); > + if (GET_MODE_PRECISION (m_mode) > GET_MODE_PRECISION (o_mode)) > + return simplify_gen_unary (SIGN_EXTEND, mode, new_shift, > + GET_MODE (new_shift)); > + if (mode != GET_MODE (new_shift)) > + return simplify_gen_unary (TRUNCATE, mode, new_shift, > + GET_MODE (new_shift)); > + return new_shift; > + } Yeah looks like it :-) You could say combine should be smarter about this, but this is a valid simplification in itself. So, okay for trunk. Thank you! Segher
On Sun, 2020-07-19 at 10:42 +0100, Roger Sayle wrote: > The combination of several my recent nvptx patches has revealed an > interesting RTL optimization opportunity. This patch to simplify-rtx.c > simplifies (sign_extend:HI (truncate:QI (?shiftrt:HI x 8))) to just > (ashiftrt:HI x 8), as the inner shift already sets the high bits > appropriately. The equivalent zero_extend variant appears to already > be implemented in simplify_unary_operation_1. > > During the compilation of one of the tests in the test suite, we > manage the generate the redundant sequence of instructions: > > (insn 17 16 18 3 (set (reg:HI 35) > (ashiftrt:HI (reg:HI 34 [ arg ]) > (const_int 8 [0x8]))) "v2si-cvt.c":14:8 94 {ashrhi3} > (expr_list:REG_DEAD (reg:HI 34 [ arg ]) > (nil))) > (insn 18 17 19 3 (set (reg:QI 36) > (truncate:QI (reg:HI 35))) "v2si-cvt.c":14:8 28 {trunchiqi2} > (expr_list:REG_DEAD (reg:HI 35) > (nil))) > (insn 19 18 20 3 (set (reg:HI 37) > (sign_extend:HI (reg:QI 36))) "v2si-cvt.c":14:6 22 {extendqihi2} > (expr_list:REG_DEAD (reg:QI 36) > (nil))) I can't recall the target, but I've seen similar sequences as well. Thanks for digging into it. Jeff
diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c index e631da4..e3630c9 100644 --- a/gcc/simplify-rtx.c +++ b/gcc/simplify-rtx.c @@ -1527,6 +1527,38 @@ simplify_unary_operation_1 (enum rtx_code code, machine_mode mode, rtx op) && XEXP (op, 1) != const0_rtx) return simplify_gen_unary (ZERO_EXTEND, mode, op, GET_MODE (op)); + /* (sign_extend:M (truncate:N (lshiftrt:O <X> (const_int I)))) where + I is GET_MODE_PRECISION(O) - GET_MODE_PRECISION(N), simplifies to + (ashiftrt:M <X> (const_int I)) if modes M and O are the same, and + (truncate:M (ashiftrt:O <X> (const_int I))) if M is narrower than + O, and (sign_extend:M (ashiftrt:O <X> (const_int I))) if M is + wider than O. */ + if (GET_CODE (op) == TRUNCATE + && GET_CODE (XEXP (op, 0)) == LSHIFTRT + && CONST_INT_P (XEXP (XEXP (op, 0), 1))) + { + scalar_int_mode m_mode, n_mode, o_mode; + rtx old_shift = XEXP (op, 0); + if (is_a <scalar_int_mode> (mode, &m_mode) + && is_a <scalar_int_mode> (GET_MODE (op), &n_mode) + && is_a <scalar_int_mode> (GET_MODE (old_shift), &o_mode) + && GET_MODE_PRECISION (o_mode) - GET_MODE_PRECISION (n_mode) + == INTVAL (XEXP (old_shift, 1))) + { + rtx new_shift = simplify_gen_binary (ASHIFTRT, + GET_MODE (old_shift), + XEXP (old_shift, 0), + XEXP (old_shift, 1)); + if (GET_MODE_PRECISION (m_mode) > GET_MODE_PRECISION (o_mode)) + return simplify_gen_unary (SIGN_EXTEND, mode, new_shift, + GET_MODE (new_shift)); + if (mode != GET_MODE (new_shift)) + return simplify_gen_unary (TRUNCATE, mode, new_shift, + GET_MODE (new_shift)); + return new_shift; + } + } + #if defined(POINTERS_EXTEND_UNSIGNED) /* As we do not know which address space the pointer is referring to, we can do this only if the target does not support different pointer