From f351abd67980aced3a656bb5cc639eda1482439f Mon Sep 17 00:00:00 2001 From: Thomas Preud'homme Date: Wed, 12 Nov 2014 09:50:20 +0000 Subject: [PATCH] [multiple changes] 2014-11-12 Thomas Preud'homme gcc/ PR tree-optimization/63761 * tree-ssa-math-opts.c (bswap_replace): Construct gsi from cur_stmt rather than taking it as a parameter. Add some comments to explain the gsi_move_before in case of load and why canonicalization of bswap into a rotation is only done for 16bit values. (pass_optimize_bswap::execute): Adapt for loop via gsi to make gsi refer to the statement just before cur_stmt. Ignore 16bit bswap that are already in canonical form. Adapt bswap_replace to removal of its gsi parameter. 2014-11-12 Thomas Preud'homme gcc/testsuite/ PR tree-optimization/63761 * gcc.c-torture/compile/pr63761.c: New test. From-SVN: r217409 --- gcc/ChangeLog | 12 ++++ gcc/testsuite/ChangeLog | 5 ++ gcc/testsuite/gcc.c-torture/compile/pr63761.c | 17 ++++++ gcc/tree-ssa-math-opts.c | 58 +++++++++++++------ 4 files changed, 74 insertions(+), 18 deletions(-) create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr63761.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index fc8510dc972..d6c7d383db6 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,15 @@ +2014-11-12 Thomas Preud'homme + + PR tree-optimization/63761 + * tree-ssa-math-opts.c (bswap_replace): Construct gsi from cur_stmt + rather than taking it as a parameter. Add some comments to explain the + gsi_move_before in case of load and why canonicalization of bswap into + a rotation is only done for 16bit values. + (pass_optimize_bswap::execute): Adapt for loop via gsi to make gsi + refer to the statement just before cur_stmt. Ignore 16bit bswap that + are already in canonical form. Adapt bswap_replace to removal of its + gsi parameter. + 2014-11-12 Richard Sandiford * rtl.h (rtx_function, for_each_rtx, for_each_rtx_in_insn): Delete. diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 1cb9e1cc36f..10c4e6f9faf 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2014-11-12 Thomas Preud'homme + + PR tree-optimization/63761 + * gcc.c-torture/compile/pr63761.c: New test. + 2014-11-12 Jiong Wang * lib/gcc-dg.exp (${tool}_load): Truncate gcc output. diff --git a/gcc/testsuite/gcc.c-torture/compile/pr63761.c b/gcc/testsuite/gcc.c-torture/compile/pr63761.c new file mode 100644 index 00000000000..5cda3f1bd8b --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/compile/pr63761.c @@ -0,0 +1,17 @@ +int a, b; +short c; + +void fn1 (); + +void +fn2 (unsigned short p1) +{ + int d; + + c = p1 >> 8 | p1 << 8; + d = b; + if (d) + fn1 (); + a = d >> 8 & 0x00FF + | d << 8 & 0xFF00; +} diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c index 12fa4e8ca10..aab056c338e 100644 --- a/gcc/tree-ssa-math-opts.c +++ b/gcc/tree-ssa-math-opts.c @@ -2172,23 +2172,28 @@ public: }; // class pass_optimize_bswap -/* Perform the bswap optimization: replace the statement CUR_STMT at - GSI with a load of type, VUSE and set-alias as described by N if a - memory source is involved (N->base_addr is non null), followed by - the builtin bswap invocation in FNDECL if BSWAP is true. SRC_STMT - gives where should the replacement be made. It also gives the - source on which CUR_STMT is operating via its rhs's first tree nad - N->range gives the size of the expression involved for maintaining - some statistics. */ +/* Perform the bswap optimization: replace the expression computed in the rhs + of CUR_STMT by an equivalent bswap, load or load + bswap expression. + Which of these alternatives replace the rhs is given by N->base_addr (non + null if a load is needed) and BSWAP. The type, VUSE and set-alias of the + load to perform are also given in N while the builtin bswap invoke is given + in FNDEL. Finally, if a load is involved, SRC_STMT refers to one of the + load statements involved to construct the rhs in CUR_STMT and N->range gives + the size of the rhs expression for maintaining some statistics. + + Note that if the replacement involve a load, CUR_STMT is moved just after + SRC_STMT to do the load with the same VUSE which can lead to CUR_STMT + changing of basic block. */ static bool -bswap_replace (gimple cur_stmt, gimple_stmt_iterator gsi, gimple src_stmt, - tree fndecl, tree bswap_type, tree load_type, - struct symbolic_number *n, bool bswap) +bswap_replace (gimple cur_stmt, gimple src_stmt, tree fndecl, tree bswap_type, + tree load_type, struct symbolic_number *n, bool bswap) { + gimple_stmt_iterator gsi; tree src, tmp, tgt; gimple bswap_stmt; + gsi = gsi_for_stmt (cur_stmt); src = gimple_assign_rhs1 (src_stmt); tgt = gimple_assign_lhs (cur_stmt); @@ -2207,6 +2212,9 @@ bswap_replace (gimple cur_stmt, gimple_stmt_iterator gsi, gimple src_stmt, && SLOW_UNALIGNED_ACCESS (TYPE_MODE (load_type), align)) return false; + /* Move cur_stmt just before one of the load of the original + to ensure it has the same VUSE. See PR61517 for what could + go wrong. */ gsi_move_before (&gsi, &gsi_ins); gsi = gsi_for_stmt (cur_stmt); @@ -2293,7 +2301,10 @@ bswap_replace (gimple cur_stmt, gimple_stmt_iterator gsi, gimple src_stmt, tmp = src; - /* Canonical form for 16 bit bswap is a rotate expression. */ + /* Canonical form for 16 bit bswap is a rotate expression. Only 16bit values + are considered as rotation of 2N bit values by N bits is generally not + equivalent to a bswap. Consider for instance 0x01020304 >> 16 which gives + 0x03040102 while a bswap for that value is 0x04030201. */ if (bswap && n->range == 16) { tree count = build_int_cst (NULL, BITS_PER_UNIT); @@ -2393,10 +2404,10 @@ pass_optimize_bswap::execute (function *fun) gimple_stmt_iterator gsi; /* We do a reverse scan for bswap patterns to make sure we get the - widest match. As bswap pattern matching doesn't handle - previously inserted smaller bswap replacements as sub- - patterns, the wider variant wouldn't be detected. */ - for (gsi = gsi_last_bb (bb); !gsi_end_p (gsi); gsi_prev (&gsi)) + widest match. As bswap pattern matching doesn't handle previously + inserted smaller bswap replacements as sub-patterns, the wider + variant wouldn't be detected. */ + for (gsi = gsi_last_bb (bb); !gsi_end_p (gsi);) { gimple src_stmt, cur_stmt = gsi_stmt (gsi); tree fndecl = NULL_TREE, bswap_type = NULL_TREE, load_type; @@ -2404,6 +2415,14 @@ pass_optimize_bswap::execute (function *fun) struct symbolic_number n; bool bswap; + /* This gsi_prev (&gsi) is not part of the for loop because cur_stmt + might be moved to a different basic block by bswap_replace and gsi + must not points to it if that's the case. Moving the gsi_prev + there make sure that gsi points to the statement previous to + cur_stmt while still making sure that all statements are + considered in this basic block. */ + gsi_prev (&gsi); + if (!is_gimple_assign (cur_stmt)) continue; @@ -2431,6 +2450,9 @@ pass_optimize_bswap::execute (function *fun) switch (n.range) { case 16: + /* Already in canonical form, nothing to do. */ + if (code == LROTATE_EXPR || code == RROTATE_EXPR) + continue; load_type = uint16_type_node; if (bswap16_p) { @@ -2461,8 +2483,8 @@ pass_optimize_bswap::execute (function *fun) if (bswap && !fndecl) continue; - if (bswap_replace (cur_stmt, gsi, src_stmt, fndecl, bswap_type, - load_type, &n, bswap)) + if (bswap_replace (cur_stmt, src_stmt, fndecl, bswap_type, load_type, + &n, bswap)) changed = true; } }