diff mbox

On x86 allow if-conversion of more than one insn as long as there is at most one cmov (PR tree-optimization/79390)

Message ID yddvaqhvgx8.fsf@CeBiTec.Uni-Bielefeld.DE
State New
Headers show

Commit Message

Rainer Orth April 6, 2017, 12:50 p.m. UTC 
Jeff Law <law@redhat.com> writes:

> On 04/01/2017 06:20 AM, Jakub Jelinek wrote:
>> Hi!
>>
>> As discussed in the PR, in the following testcase we don't if-convert
>> with the generic (and many other) tuning, because we default to
>> --param max-rtl-if-conversion-insns=1 in most of the tunings.
>> The problem we have is with multiple cmov instructions, but in the
>> testcase there is just one cmov and the other insn is turned into a SSE
>> max insn, which is fine.
>>
>> This patch stops artificially lowering that param, and for one_if_conv_insn
>> tuning it instead rejects the if-conversion if the resulting sequence has
>> multiple cmov instructions.  The hook is passed if_info too, so it can
>> in the future do better heuristics based on predictability of the edges,
>> how far the uses of the cmov result are (I assume cmov major problem is
>> latency, right?) etc.
>>
>> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>>
>> 2017-04-01  Jakub Jelinek  <jakub@redhat.com>
>>
>> 	PR tree-optimization/79390
>> 	* target.h (struct noce_if_info): Declare.
>> 	* targhooks.h (default_noce_conversion_profitable_p): Declare.
>> 	* target.def (noce_conversion_profitable_p): New target hook.
>> 	* ifcvt.h (struct noce_if_info): New type, moved from ...
>> 	* ifcvt.c (struct noce_if_info): ... here.
>> 	(noce_conversion_profitable_p): Renamed to ...
>> 	(default_noce_conversion_profitable_p): ... this.  No longer
>> 	static nor inline.
>> 	(noce_try_store_flag_constants, noce_try_addcc,
>> 	noce_try_store_flag_mask, noce_try_cmove, noce_try_cmove_arith,
>> 	noce_convert_multiple_sets): Use targetm.noce_conversion_profitable_p
>> 	instead of noce_conversion_profitable_p.
>> 	* config/i386/i386.c: Include ifcvt.h.
>> 	(ix86_option_override_internal): Don't override
>> 	PARAM_MAX_RTL_IF_CONVERSION_INSNS default.
>> 	(ix86_noce_conversion_profitable_p): New function.
>> 	(TARGET_NOCE_CONVERSION_PROFITABLE_P): Redefine.
>> 	* config/i386/x86-tune.def (X86_TUNE_ONE_IF_CONV_INSN): Adjust comment.
>> 	* doc/tm.texi.in (TARGET_NOCE_CONVERSION_PROFITABLE_P): Add.
>> 	* doc/tm.texi: Regenerated.
>>
>> 	* gcc.target/i386/pr79390.c: New test.
>> 	* gcc.dg/ifcvt-4.c: Use -mtune-ctrl=^one_if_conv_insn for i?86/x86_64.
> OK.

the new test FAILs on Solaris/x86 with /bin/as:

FAIL: gcc.target/i386/pr79390.c scan-assembler [ \\\\t]cmov[a-z]+[ \\\\t]

That's because gcc emits

        cmovl.a %edx, %eax

instead of

        cmova   %edx, %eax

Fixed as follows, tested with the appropriate runtest invocations on
i386-pc-solaris2.12 and x86_64-pc-linux-gnu.

I guess this is obvious?

Thanks.
        Rainer

Comments

Jakub Jelinek April 6, 2017, 12:55 p.m. UTC | #1 
On Thu, Apr 06, 2017 at 02:50:27PM +0200, Rainer Orth wrote:
> 2017-04-06  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
> 
> 	* gcc.target/i386/pr79390.c: Allow for cmovl.a.

Please mention PR tree-optimization/79390 in the ChangeLog
and commit message.  Ok with that change.

> # HG changeset patch
> # Parent  7c92d635959dcb1a757b301344d8519dde9e1e7a
> Fix gcc.target/i386/pr79390.c for Solaris as
> 
> diff --git a/gcc/testsuite/gcc.target/i386/pr79390.c b/gcc/testsuite/gcc.target/i386/pr79390.c
> --- a/gcc/testsuite/gcc.target/i386/pr79390.c
> +++ b/gcc/testsuite/gcc.target/i386/pr79390.c
> @@ -25,4 +25,4 @@ foo (void)
>    return jp;
>  }
>  
> -/* { dg-final { scan-assembler "\[ \\t\]cmov\[a-z]+\[ \\t\]" } } */
> +/* { dg-final { scan-assembler "\[ \\t\]cmov\[a-z.]+\[ \\t\]" } } */

	Jakub
diff mbox

Patch

# HG changeset patch
# Parent  7c92d635959dcb1a757b301344d8519dde9e1e7a
Fix gcc.target/i386/pr79390.c for Solaris as

diff --git a/gcc/testsuite/gcc.target/i386/pr79390.c b/gcc/testsuite/gcc.target/i386/pr79390.c
--- a/gcc/testsuite/gcc.target/i386/pr79390.c
+++ b/gcc/testsuite/gcc.target/i386/pr79390.c
@@ -25,4 +25,4 @@  foo (void)
   return jp;
 }
 
-/* { dg-final { scan-assembler "\[ \\t\]cmov\[a-z]+\[ \\t\]" } } */
+/* { dg-final { scan-assembler "\[ \\t\]cmov\[a-z.]+\[ \\t\]" } } */