| Message ID | 561F3E61.7060906@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Thu, Oct 15, 2015 at 7:49 AM, Kugan <kugan.vivekanandarajah@linaro.org> wrote: > > > On 12/10/15 23:21, Richard Biener wrote: >> On Sun, Oct 11, 2015 at 12:35 PM, Kugan >> <kugan.vivekanandarajah@linaro.org> wrote: >>> >>> >>> On 15/09/15 23:18, Richard Biener wrote: >>>> On Mon, Sep 7, 2015 at 4:55 AM, Kugan <kugan.vivekanandarajah@linaro.org> wrote: >>>>> >>>>> This patch adds support for new tree code SEXT_EXPR. >>>> >>>> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c >>>> index d567a87..bbc3c10 100644 >>>> --- a/gcc/cfgexpand.c >>>> +++ b/gcc/cfgexpand.c >>>> @@ -5071,6 +5071,10 @@ expand_debug_expr (tree exp) >>>> case FMA_EXPR: >>>> return simplify_gen_ternary (FMA, mode, inner_mode, op0, op1, op2); >>>> >>>> + case SEXT_EXPR: >>>> + return op0; >>>> >>>> that looks wrong. Generate (sext:... ) here? >>>> >>>> + case SEXT_EXPR: >>>> + { >>>> + rtx op0 = expand_normal (treeop0); >>>> + rtx temp; >>>> + if (!target) >>>> + target = gen_reg_rtx (TYPE_MODE (TREE_TYPE (treeop0))); >>>> + >>>> + machine_mode inner_mode >>>> + = smallest_mode_for_size (tree_to_shwi (treeop1), >>>> + MODE_INT); >>>> + temp = convert_modes (inner_mode, >>>> + TYPE_MODE (TREE_TYPE (treeop0)), op0, 0); >>>> + convert_move (target, temp, 0); >>>> + return target; >>>> + } >>>> >>>> Humm - is that really how we expand sign extensions right now? No helper >>>> that would generate (sext ...) directly? I wouldn't try using 'target' btw but >>>> simply return (sext:mode op0 op1) or so. But I am no way an RTL expert. >>>> >>>> Note that if we don't disallow arbitrary precision SEXT_EXPRs we have to >>>> fall back to using shifts (and smallest_mode_for_size is simply wrong). >>>> >>>> + case SEXT_EXPR: >>>> + { >>>> + if (!INTEGRAL_TYPE_P (lhs_type) >>>> + || !INTEGRAL_TYPE_P (rhs1_type) >>>> + || TREE_CODE (rhs2) != INTEGER_CST) >>>> >>>> please constrain this some more, with >>>> >>>> || !useless_type_conversion_p (lhs_type, rhs1_type) >>>> >>>> + { >>>> + error ("invalid operands in sext expr"); >>>> + return true; >>>> + } >>>> + return false; >>>> + } >>>> >>>> @@ -3414,6 +3422,9 @@ op_symbol_code (enum tree_code code) >>>> case MIN_EXPR: >>>> return "min"; >>>> >>>> + case SEXT_EXPR: >>>> + return "sext from bit"; >>>> + >>>> >>>> just "sext" please. >>>> >>>> +/* Sign-extend operation. It will sign extend first operand from >>>> + the sign bit specified by the second operand. */ >>>> +DEFTREECODE (SEXT_EXPR, "sext_expr", tcc_binary, 2) >>>> >>>> "from the INTEGER_CST sign bit specified" >>>> >>>> Also add "The type of the result is that of the first operand." >>>> >>> >>> >>> >>> Thanks for the review. Attached patch attempts to address the above >>> comments. Does this look better? >> >> + case SEXT_EXPR: >> + gcc_assert (CONST_INT_P (op1)); >> + inner_mode = mode_for_size (INTVAL (op1), MODE_INT, 0); >> >> We should add >> >> gcc_assert (GET_MODE_BITSIZE (inner_mode) == INTVAL (op1)); >> >> + if (mode != inner_mode) >> + op0 = simplify_gen_unary (SIGN_EXTEND, >> + mode, >> + gen_lowpart_SUBREG (inner_mode, op0), >> + inner_mode); >> >> as we're otherwise silently dropping things like SEXT (short-typed-var, 13) >> >> + case SEXT_EXPR: >> + { >> + machine_mode inner_mode = mode_for_size (tree_to_shwi (treeop1), >> + MODE_INT, 0); >> >> Likewise. Also treeop1 should be unsigned, thus tree_to_uhwi? >> >> + rtx temp, result; >> + rtx op0 = expand_normal (treeop0); >> + op0 = force_reg (mode, op0); >> + if (mode != inner_mode) >> + { >> >> Again, for the RTL bits I'm not sure they are correct. For example I don't >> see why we need a lowpart SUBREG, isn't a "regular" SUBREG enough? >> >> + case SEXT_EXPR: >> + { >> + if (!INTEGRAL_TYPE_P (lhs_type) >> + || !useless_type_conversion_p (lhs_type, rhs1_type) >> + || !INTEGRAL_TYPE_P (rhs1_type) >> + || TREE_CODE (rhs2) != INTEGER_CST) >> >> the INTEGRAL_TYPE_P (rhs1_type) check is redundant with >> the useless_type_Conversion_p one. Please check >> tree_fits_uhwi (rhs2) instead of != INTEGER_CST. >> > > Thanks for the review, Please find the updated patch based on the review > comments. Ok. Thanks, Richard. > Thanks, > Kugan
From e600c266ac7932ffe4cb36830e8d62c90f6e26ee Mon Sep 17 00:00:00 2001 From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org> Date: Mon, 17 Aug 2015 13:37:15 +1000 Subject: [PATCH 1/7] Add new SEXT_EXPR tree code --- gcc/cfgexpand.c | 12 ++++++++++++ gcc/expr.c | 20 ++++++++++++++++++++ gcc/fold-const.c | 4 ++++ gcc/tree-cfg.c | 12 ++++++++++++ gcc/tree-inline.c | 1 + gcc/tree-pretty-print.c | 11 +++++++++++ gcc/tree.def | 5 +++++ 7 files changed, 65 insertions(+) diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index 58e55d2..357710b 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -5057,6 +5057,18 @@ expand_debug_expr (tree exp) case FMA_EXPR: return simplify_gen_ternary (FMA, mode, inner_mode, op0, op1, op2); + case SEXT_EXPR: + gcc_assert (CONST_INT_P (op1)); + inner_mode = mode_for_size (INTVAL (op1), MODE_INT, 0); + gcc_assert (GET_MODE_BITSIZE (inner_mode) == INTVAL (op1)); + + if (mode != inner_mode) + op0 = simplify_gen_unary (SIGN_EXTEND, + mode, + gen_lowpart_SUBREG (inner_mode, op0), + inner_mode); + return op0; + default: flag_unsupported: #ifdef ENABLE_CHECKING diff --git a/gcc/expr.c b/gcc/expr.c index 0bbfccd..63bd1b6 100644 --- a/gcc/expr.c +++ b/gcc/expr.c @@ -9296,6 +9296,26 @@ expand_expr_real_2 (sepops ops, rtx target, machine_mode tmode, target = expand_vec_cond_expr (type, treeop0, treeop1, treeop2, target); return target; + case SEXT_EXPR: + { + machine_mode inner_mode = mode_for_size (tree_to_uhwi (treeop1), + MODE_INT, 0); + rtx temp, result; + rtx op0 = expand_normal (treeop0); + op0 = force_reg (mode, op0); + if (mode != inner_mode) + { + result = gen_reg_rtx (mode); + temp = simplify_gen_unary (SIGN_EXTEND, mode, + gen_lowpart_SUBREG (inner_mode, op0), + inner_mode); + convert_move (result, temp, 0); + } + else + result = op0; + return result; + } + default: gcc_unreachable (); } diff --git a/gcc/fold-const.c b/gcc/fold-const.c index 7231fd6..d693b42 100644 --- a/gcc/fold-const.c +++ b/gcc/fold-const.c @@ -984,6 +984,10 @@ int_const_binop_1 (enum tree_code code, const_tree arg1, const_tree parg2, res = wi::bit_and (arg1, arg2); break; + case SEXT_EXPR: + res = wi::sext (arg1, arg2.to_uhwi ()); + break; + case RSHIFT_EXPR: case LSHIFT_EXPR: if (wi::neg_p (arg2)) diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c index 712d8cc..03ae758 100644 --- a/gcc/tree-cfg.c +++ b/gcc/tree-cfg.c @@ -3756,6 +3756,18 @@ verify_gimple_assign_binary (gassign *stmt) return false; } + case SEXT_EXPR: + { + if (!INTEGRAL_TYPE_P (lhs_type) + || !useless_type_conversion_p (lhs_type, rhs1_type) + || !tree_fits_uhwi_p (rhs2)) + { + error ("invalid operands in sext expr"); + return true; + } + return false; + } + case VEC_WIDEN_LSHIFT_HI_EXPR: case VEC_WIDEN_LSHIFT_LO_EXPR: { diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c index ac9586e..0975730 100644 --- a/gcc/tree-inline.c +++ b/gcc/tree-inline.c @@ -3884,6 +3884,7 @@ estimate_operator_cost (enum tree_code code, eni_weights *weights, case BIT_XOR_EXPR: case BIT_AND_EXPR: case BIT_NOT_EXPR: + case SEXT_EXPR: case TRUTH_ANDIF_EXPR: case TRUTH_ORIF_EXPR: diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c index 7cd1fe7..efd8d5b 100644 --- a/gcc/tree-pretty-print.c +++ b/gcc/tree-pretty-print.c @@ -1794,6 +1794,14 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, int flags, } break; + case SEXT_EXPR: + pp_string (pp, "SEXT_EXPR <"); + dump_generic_node (pp, TREE_OPERAND (node, 0), spc, flags, false); + pp_string (pp, ", "); + dump_generic_node (pp, TREE_OPERAND (node, 1), spc, flags, false); + pp_greater (pp); + break; + case MODIFY_EXPR: case INIT_EXPR: dump_generic_node (pp, TREE_OPERAND (node, 0), spc, flags, @@ -3414,6 +3422,9 @@ op_symbol_code (enum tree_code code) case MIN_EXPR: return "min"; + case SEXT_EXPR: + return "sext"; + default: return "<<< ??? >>>"; } diff --git a/gcc/tree.def b/gcc/tree.def index 56580af..48e7413 100644 --- a/gcc/tree.def +++ b/gcc/tree.def @@ -752,6 +752,11 @@ DEFTREECODE (BIT_XOR_EXPR, "bit_xor_expr", tcc_binary, 2) DEFTREECODE (BIT_AND_EXPR, "bit_and_expr", tcc_binary, 2) DEFTREECODE (BIT_NOT_EXPR, "bit_not_expr", tcc_unary, 1) +/* Sign-extend operation. It will sign extend first operand from + the sign bit specified by the second operand. The type of the + result is that of the first operand. */ +DEFTREECODE (SEXT_EXPR, "sext_expr", tcc_binary, 2) + /* ANDIF and ORIF allow the second operand not to be computed if the value of the expression is determined from the first operand. AND, OR, and XOR always compute the second operand whether its value is -- 1.9.1