From: Kyrill Tkachov <kyrylo.tkachov@arm.com>
To: Rainer Orth <ro@cebitec.uni-bielefeld.de>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>, Jeff Law <law@redhat.com>
Subject: Re: [PATCH][RTL-ifcvt] PR rtl-optimization/67465: Handle pairs of complex+simple blocks and empty blocks more gracefully
Date: Fri, 11 Sep 2015 15:43:00 -0000 [thread overview]
Message-ID: <55F2F575.6080609@arm.com> (raw)
In-Reply-To: <yddr3m5bap3.fsf@lokon.CeBiTec.Uni-Bielefeld.DE>
[-- Attachment #1: Type: text/plain, Size: 2994 bytes --]
On 11/09/15 09:51, Rainer Orth wrote:
> Kyrill Tkachov <kyrylo.tkachov@arm.com> writes:
>
>> On 10/09/15 12:43, Rainer Orth wrote:
>>> Hi Kyrill,
>>>
>>>> Rainer, could you please check that this patch still fixes the SPARC
>>>> regressions?
>>> unfortunately, it breaks sparc-sun-solaris2.10 bootstrap: compiling
>>> stage2 libiberty/regex.c FAILs:
>>>
>>>
>> Thanks for providing the preprocessed file.
>> I've reproduced and fixed the ICE in this version of the patch.
>> The problem was that I was taking the mode of x before the check
>> of whether a and b are MEMs, after which we would change x to an
>> address_mode reg,
>> thus confusing emit_move_insn.
>>
>> The fix is to take the mode of x and perform the can_conditionally_move_p check
>> after that transformation.
>>
>> Bootstrapped and tested on aarch64 and x86_64.
>> The preprocessed regex.i that Rainer provided now compiles successfully for me
>> on a sparc-sun-solaris2.10 stage-1 cross-compiler.
>>
>> Rainer, thanks for your help so far, could you please try out this patch?
> While bootstrap succeeds again, the testsuite regression in
> gcc.c-torture/execute/20071216-1.c reoccured.
Right, so I dug into the RTL dumps and I think this is a separate issue that's being exacerbated by my patch.
The code tries to if-convert a block which contains a compare instruction i.e. sets the CC register.
Now, bb_valid_for_noce_process_p should have caught this, and in particular insn_valid_noce_process_p
which should check that the instruction doesn't set the CC register. However, on SPARC the
cc_in_cond returns NULL! This is due to the canonicalize_comparison implementation that seems to
remove the CC register from the condition expression and returns something like:
(leu (reg/v:SI 109 [ b ])
(const_int -4096 [0xfffffffffffff000])
Therefore the set_of (cc_in_cond (cond), insn) check doesn't get triggered because cc_in_cond returns NULL.
Regardless of how the branch condition got canonicalized, I think we still want to reject any insn in the block
that sets a condition code register, so this patch checks the destination of every set in the block for a MODE_CC
expression and cancels if-conversion if that's the case.
Oleg pointed me to the older PR 58517 affecting SH which seems similar and I think my previous ifcvt patch would expose
this problem more.
Anyway, with this patch the failing SPARC testcase gcc.c-torture/execute/20071216-1.c generates the same assembly
as before r227368 and bootstrap and test on aarch64 and x86_64 passes ok for me.
Rainer, could you try this patch on top of the previous patch? (https://gcc.gnu.org/ml/gcc-patches/2015-09/msg00689.html)
The two together should fix all of PR 67456, 67464, 67465 and 67481.
Thanks,
Kyrill
2015-09-11 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
PR rtl-optimization/67481
* ifcvt.c (contains_ccmode_rtx_p): New function.
(insn_valid_noce_process_p): Use it.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ifcvt-bug-2.patch --]
[-- Type: text/x-patch; name=ifcvt-bug-2.patch, Size: 1004 bytes --]
diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index 9af3249..090a584 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -1838,6 +1838,19 @@ noce_try_cmove (struct noce_if_info *if_info)
return FALSE;
}
+/* Return true if X contains a conditional code mode rtx. */
+
+static bool
+contains_ccmode_rtx_p (rtx x)
+{
+ subrtx_iterator::array_type array;
+ FOR_EACH_SUBRTX (iter, array, x, ALL)
+ if (GET_MODE_CLASS (GET_MODE (*iter)) == MODE_CC)
+ return true;
+
+ return false;
+}
+
/* Helper for bb_valid_for_noce_process_p. Validate that
the rtx insn INSN is a single set that does not set
the conditional register CC and is in general valid for
@@ -1856,6 +1869,7 @@ insn_valid_noce_process_p (rtx_insn *insn, rtx cc)
/* Currently support only simple single sets in test_bb. */
if (!sset
|| !noce_operand_ok (SET_DEST (sset))
+ || contains_ccmode_rtx_p (SET_DEST (sset))
|| !noce_operand_ok (SET_SRC (sset)))
return false;
next prev parent reply other threads:[~2015-09-11 15:38 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-10 8:23 Kyrill Tkachov
2015-09-10 11:57 ` Rainer Orth
2015-09-10 13:18 ` Kyrill Tkachov
2015-09-10 14:52 ` Kyrill Tkachov
2015-09-11 8:53 ` Rainer Orth
2015-09-11 15:43 ` Kyrill Tkachov [this message]
2015-09-17 12:02 ` Rainer Orth
2015-09-17 16:36 ` Kyrill Tkachov
2015-09-18 9:24 ` Rainer Orth
2015-09-25 11:20 ` Rainer Orth
2015-09-25 11:21 ` Kyrill Tkachov
2015-09-25 20:32 ` Jeff Law
2015-09-28 9:43 ` Kyrill Tkachov
2015-09-28 17:09 ` H.J. Lu
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=55F2F575.6080609@arm.com \
--to=kyrylo.tkachov@arm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=law@redhat.com \
--cc=ro@cebitec.uni-bielefeld.de \
/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).