public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Georg-Johann Lay <avr@gjlay.de>
To: gcc-patches@gcc.gnu.org
Cc: Jeff Law <jeffreyalaw@gmail.com>, Denis Chertykov <chertykov@gmail.com>
Subject: Ping #2: [patch,avr] Fix PR109650 wrong code
Date: Wed, 7 Jun 2023 10:39:16 +0200	[thread overview]
Message-ID: <2805e6e2-b891-9a9c-64c1-cd5ee4ca53da@gjlay.de> (raw)
In-Reply-To: <fc1cb714-83b4-4981-443b-d9b8d4a72c00@gjlay.de>

Ping #2 for:

https://gcc.gnu.org/pipermail/gcc-patches/2023-May/618976.html

https://gcc.gnu.org/pipermail/gcc-patches/attachments/20230519/9536bf8c/attachment-0001.bin

Ping #1:
https://gcc.gnu.org/pipermail/gcc-patches/2023-May/620098.html

Johann

Am 19.05.23 um 10:49 schrieb Georg-Johann Lay:
> 
> Here is a revised version of the patch.  The difference to the
> previous one is that it adds some combine patterns for *cbranch
> insns that were lost in the PR92729 transition.  The post-reload
> part of the patterns were still there.  The new patterns are
> slightly more general in that they also handle fixed-point modes.
> 
> Apart from that, the patch behaves the same:
> 
> Am 15.05.23 um 20:05 schrieb Georg-Johann Lay:
>> This patch fixes a wrong-code bug in the wake of PR92729, the transition
>> that turned the AVR backend from cc0 to CCmode.  In cc0, the insn that
>> uses cc0 like a conditional branch always follows the cc0 setter, which
>> is no more the case with CCmode where set and use of REG_CC might be in
>> different basic blocks.
>>
>> This patch removes the machine-dependent reorg pass in avr_reorg 
>> entirely.
>>
>> It is replaced by a new, AVR specific mini-pass that runs prior to
>> split2. Canonicalization of comparisons away from the "difficult"
>> codes GT[U] and LE[U] is now mostly performed by implementing
>> TARGET_CANONICALIZE_COMPARISON.
>>
>> Moreover:
>>
>> * Text peephole conditions get "dead_or_set_regno_p (*, REG_CC)" as
>> needed.
>>
>> * RTL peephole conditions get "peep2_regno_dead_p (*, REG_CC)" as
>> needed.
>>
>> * Conditional branches no more clobber REG_CC.
>>
>> * insn output for compares looks ahead to determine the branch mode in
>> use. This needs also "dead_or_set_regno_p (*, REG_CC)".
>>
>> * Add RTL peepholes for decrement-and-branch detection.
>>
>> Finally, it fixes some of the many indentation glitches left over from
>> PR92729.
>>
>> Ok?
>>
>> I'd also backport this one because all of v12+ is affected by the 
>> wrong code.
> 
> Johann
> 
> -- 
> 
> gcc/
>      PR target/109650
>      PR target/92729
> 
>      * config/avr/avr-passes.def (avr_pass_ifelse): Insert new pass.
>      * config/avr/avr.cc (avr_pass_ifelse): New RTL pass.
>      (avr_pass_data_ifelse): New pass_data for it.
>      (make_avr_pass_ifelse, avr_redundant_compare, avr_cbranch_cost)
>      (avr_canonicalize_comparison, avr_out_plus_set_ZN)
>      (avr_out_cmp_ext): New functions.
>      (compare_condtition): Make sure REG_CC dies in the branch insn.
>      (avr_rtx_costs_1): Add computation of cbranch costs.
>      (avr_adjust_insn_length) [ADJUST_LEN_ADD_SET_ZN, ADJUST_LEN_CMP_ZEXT]:
>      [ADJUST_LEN_CMP_SEXT]Handle them.
>      (TARGET_CANONICALIZE_COMPARISON): New define.
>      (avr_simplify_comparison_p, compare_diff_p, avr_compare_pattern)
>      (avr_reorg_remove_redundant_compare, avr_reorg): Remove functions.
>      (TARGET_MACHINE_DEPENDENT_REORG): Remove define.
> 
>      * avr-protos.h (avr_simplify_comparison_p): Remove proto.
>      (make_avr_pass_ifelse, avr_out_plus_set_ZN, cc_reg_rtx)
>      (avr_out_cmp_zext): New Protos
> 
>      * config/avr/avr.md (branch, difficult_branch): Don't split insns.
>      (*cbranchhi.zero-extend.0", *cbranchhi.zero-extend.1")
>      (*swapped_tst<mode>, *add.for.eqne.<mode>): New insns.
>      (*cbranch<mode>4): Rename to cbranch<mode>4_insn.
>      (define_peephole): Add dead_or_set_regno_p(insn,REG_CC) as needed.
>      (define_deephole2): Add peep2_regno_dead_p(*,REG_CC) as needed.
>      Add new RTL peepholes for decrement-and-branch and *swapped_tst<mode>.
>      Rework signtest-and-branch peepholes for *sbrx_branch<mode>.
>      (adjust_len) [add_set_ZN, cmp_zext]: New.
>      (QIPSI): New mode iterator.
>      (ALLs1, ALLs2, ALLs4, ALLs234): New mode iterators.
>      (gelt): New code iterator.
>      (gelt_eqne): New code attribute.
>      (rvbranch, *rvbranch, difficult_rvbranch, *difficult_rvbranch)
>      (branch_unspec, *negated_tst<mode>, *reversed_tst<mode>)
>      (*cmpqi_sign_extend): Remove insns.
>      (define_c_enum "unspec") [UNSPEC_IDENTITY]: Remove.
> 
>      * config/avr/avr-dimode.md (cbranch<mode>4): Canonicalize comparisons.
>      * config/avr/predicates.md (scratch_or_d_register_operand): New.
>      * config/avr/contraints.md (Yxx): New constraint.
> 
> gcc/testsuite/
>      PR target/109650
>      * config/avr/torture/pr109650-1.c: New test.
>      * config/avr/torture/pr109650-2.c: New test.

  parent reply	other threads:[~2023-06-07  8:39 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-15 18:05 Georg-Johann Lay
2023-05-19  8:17 ` Georg-Johann Lay
2023-05-19  8:49 ` Georg-Johann Lay
2023-05-30 13:15   ` Ping #1: " Georg-Johann Lay
2023-06-07  8:39   ` Georg-Johann Lay [this message]
2023-06-10 16:58   ` Jeff Law

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2805e6e2-b891-9a9c-64c1-cd5ee4ca53da@gjlay.de \
    --to=avr@gjlay.de \
    --cc=chertykov@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jeffreyalaw@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).