Message ID | CAFULd4b2xTuHyQNxpdPjnxmg0OZfhmzWLAwrJskCNCZEULt8gg@mail.gmail.com |
---|---|
State | New |
Headers | show |
On 02/09/2012 03:47 PM, Uros Bizjak wrote: > 2012-02-10 Uros Bizjak <ubizjak@gmail.com> > > * compare-elim.c (find_comparisons_in_bb): Eliminate only compares > having mode compatible with the mode of previous compare. Substitute > compare mode of previous compare with the mode, compatible > with eliminated and previous compare. This patch is ok for 4.8. For 4.6 and 4.7 I would prefer that we simply not eliminate the compare. I.e. + enum machine_mode src_mode = GET_MODE (src); + /* Eliminate a compare that's redundant with the previous. */ if (last_cmp_valid + && src_mode == last_cmp->orig_mode && rtx_equal_p (last_cmp->in_a, XEXP (src, 0)) && rtx_equal_p (last_cmp->in_b, XEXP (src, 1))) { ... - last_cmp->orig_mode = GET_MODE (SET_DEST (single_set (insn))); + last_cmp->orig_mode = src_mode; For 4.6 and 4.7, there are only two extant users of this pass and neither of them use anything besides CCmode before compare-elim.c does its own manipulation of the modes later. r~
On Sat, Feb 11, 2012 at 1:26 AM, Richard Henderson <rth@redhat.com> wrote: > On 02/09/2012 03:47 PM, Uros Bizjak wrote: >> 2012-02-10 Uros Bizjak <ubizjak@gmail.com> >> >> * compare-elim.c (find_comparisons_in_bb): Eliminate only compares >> having mode compatible with the mode of previous compare. Substitute >> compare mode of previous compare with the mode, compatible >> with eliminated and previous compare. > > This patch is ok for 4.8. Unfortunately, we need to update all uses of flag register with a new, compatible mode, as well, similar to how compatible mode is handled in CSE2 pass with cse_condition_code_reg in cse.c The postreload compare elimination pass should be extended to handle compare elimination for targets that expose FLAGS_REG early through SELECT_CC_MODE, taking into account the fact that flags reg can live past BB boundaries. Uros.
On 02/11/2012 07:09 AM, Uros Bizjak wrote: > On Sat, Feb 11, 2012 at 1:26 AM, Richard Henderson <rth@redhat.com> wrote: >> On 02/09/2012 03:47 PM, Uros Bizjak wrote: >>> 2012-02-10 Uros Bizjak <ubizjak@gmail.com> >>> >>> * compare-elim.c (find_comparisons_in_bb): Eliminate only compares >>> having mode compatible with the mode of previous compare. Substitute >>> compare mode of previous compare with the mode, compatible >>> with eliminated and previous compare. >> >> This patch is ok for 4.8. > > Unfortunately, we need to update all uses of flag register with a new, > compatible mode, as well, similar to how compatible mode is handled in > CSE2 pass with cse_condition_code_reg in cse.c We do? What subsequent pass really cares? What goes wrong leaving things as they are? r~
On Sat, Feb 11, 2012 at 11:01 PM, Richard Henderson <rth@redhat.com> wrote: >>>> * compare-elim.c (find_comparisons_in_bb): Eliminate only compares >>>> having mode compatible with the mode of previous compare. Substitute >>>> compare mode of previous compare with the mode, compatible >>>> with eliminated and previous compare. >>> >>> This patch is ok for 4.8. >> >> Unfortunately, we need to update all uses of flag register with a new, >> compatible mode, as well, similar to how compatible mode is handled in >> CSE2 pass with cse_condition_code_reg in cse.c > > We do? What subsequent pass really cares? Yes, please see an example in [1] how CSE2 pass handles this. I don't know if this is necessary, but when we merge arithmetic insn with compare, we _do_ update the flags users, in the same way as CSE2 pass. > What goes wrong leaving things as they are? x86 tolerates wrong modes in flags users, so as far as x86 is concerned, this is allowed. But I don't know if wrong, although compatible modes, should be generated from a generic pass. [1] http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00547.htm Uros.
Index: compare-elim.c =================================================================== --- compare-elim.c (revision 184067) +++ compare-elim.c (working copy) @@ -297,11 +297,36 @@ find_comparisons_in_bb (struct dom_walk_data *data src = conforming_compare (insn); if (src) { + enum machine_mode src_mode = GET_MODE (src); + /* Eliminate a compare that's redundant with the previous. */ if (last_cmp_valid && rtx_equal_p (last_cmp->in_a, XEXP (src, 0)) && rtx_equal_p (last_cmp->in_b, XEXP (src, 1))) { + rtx flags, x; + enum machine_mode new_mode + = targetm.cc_modes_compatible (last_cmp->orig_mode, src_mode); + + /* New mode is incompatible with the previous compare mode. */ + if (new_mode == VOIDmode) + continue; + + if (new_mode != last_cmp->orig_mode) + { + flags = gen_rtx_REG (src_mode, targetm.flags_regnum); + + /* Generate new comparison for substitution. */ + x = gen_rtx_COMPARE (new_mode, XEXP (src, 0), XEXP (src, 1)); + x = gen_rtx_SET (VOIDmode, flags, x); + + if (!validate_change (last_cmp->insn, + &PATTERN (last_cmp->insn), x, false)) + continue; + + last_cmp->orig_mode = new_mode; + } + delete_insn (insn); continue; } @@ -311,7 +336,7 @@ find_comparisons_in_bb (struct dom_walk_data *data last_cmp->prev_clobber = last_clobber; last_cmp->in_a = XEXP (src, 0); last_cmp->in_b = XEXP (src, 1); - last_cmp->orig_mode = GET_MODE (SET_DEST (single_set (insn))); + last_cmp->orig_mode = src_mode; VEC_safe_push (comparison_struct_p, heap, all_compares, last_cmp); /* It's unusual, but be prepared for comparison patterns that Index: config/i386/i386.c =================================================================== --- config/i386/i386.c (revision 184067) +++ config/i386/i386.c (working copy) @@ -17778,6 +17778,11 @@ ix86_cc_modes_compatible (enum machine_mode m1, en || (m1 == CCGOCmode && m2 == CCGCmode)) return CCGCmode; + if (m1 == CCZmode && (m2 == CCGCmode || m2 == CCGOCmode)) + return m2; + else if (m2 == CCZmode && (m1 == CCGCmode || m1 == CCGOCmode)) + return m1; + switch (m1) { default: @@ -38685,6 +38690,9 @@ ix86_autovectorize_vector_sizes (void) #undef TARGET_EXPAND_TO_RTL_HOOK #define TARGET_EXPAND_TO_RTL_HOOK ix86_maybe_switch_abi +#undef TARGET_FLAGS_REGNUM +#define TARGET_FLAGS_REGNUM FLAGS_REG + #undef TARGET_LEGITIMATE_ADDRESS_P #define TARGET_LEGITIMATE_ADDRESS_P ix86_legitimate_address_p