public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [x86_64 PATCH] PR tree-opt/91384: peephole2 to eliminate testl after negl.
@ 2022-02-28 17:36 Roger Sayle
  2022-02-28 21:11 ` Uros Bizjak
  0 siblings, 1 reply; 2+ messages in thread
From: Roger Sayle @ 2022-02-28 17:36 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1563 bytes --]


This patch is my proposed solution to PR tree-optimization/91384 which is
a missed-optimization/code quality regression on x86_64.  The problematic
idiom is "if (r = -a)" which is equivalent to both "r = -a; if (r != 0)"
and alternatively "r = -a; if (a != 0)".  In this particular case, on
x86_64, we prefer to use the condition codes from the negation, rather
than require an explicit testl instruction.

Unfortunately, combine can't help, as it doesn't attempt to merge pairs
of instructions that share the same operand(s), only pairs/triples of
instructions where the result of each instruction feeds the next.  But
I doubt there's sufficient benefit to attempt this kind of "combination"
(that wouldn't already be caught by the tree-ssa passes).

Fortunately, it's relatively easy to fix this up (addressing the
regression) during peephole2 to eliminate the unnecessary testl in:

        movl    %edi, %ebx
        negl    %ebx
        testl   %edi, %edi
        je      .L2

Tested on x86_64-pc-linux-gnu with make bootstrap and make -k check,
both with and without --target_board='unix{-m32\ -march=cascadelake}'
with no new failures.  Ok for mainline?


2022-02-28  Roger Sayle  <roger@nextmovesoftware.com>

gcc/ChangeLog
	PR tree-optimization/91384
	* config/i386/i386.md (peephole2): Eliminate final testl insn
	from the sequence *movsi_internal, *negsi_1, *cmpsi_ccno_1 by
	transforming using *negsi_2 for the negation.

gcc/testsuite/ChangeLog
	PR tree-optimization/91384
	* gcc.target/i386/pr91384.c: New test case.


Thanks in advance,
Roger
--


[-- Attachment #2: patchphb.txt --]
[-- Type: text/plain, Size: 1399 bytes --]

diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 8ffa641..4f082ee 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -11012,6 +11012,19 @@
   [(set_attr "type" "negnot")
    (set_attr "mode" "<MODE>")])
 
+;; Optimize *negsi_1 followed by *cmpsi_ccno_1 (PR target/91384)
+(define_peephole2
+  [(set (match_operand:SWI 0 "general_reg_operand")
+	(match_operand:SWI 1 "general_reg_operand"))
+   (parallel [(set (match_dup 0) (neg:SWI (match_dup 0)))
+	      (clobber (reg:CC FLAGS_REG))])
+   (set (reg:CCZ FLAGS_REG) (compare:CCZ (match_dup 1) (const_int 0)))]
+  ""
+  [(set (match_dup 0) (match_dup 1))
+   (parallel [(set (reg:CCZ FLAGS_REG)
+		   (compare:CCZ (neg:SWI (match_dup 0)) (const_int 0)))
+	      (set (match_dup 0) (neg:SWI (match_dup 0)))])])
+
 ;; Special expand pattern to handle integer mode abs
 
 (define_expand "abs<mode>2"
diff --git a/gcc/testsuite/gcc.target/i386/pr91384.c b/gcc/testsuite/gcc.target/i386/pr91384.c
new file mode 100644
index 0000000..24a60a9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr91384.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+void foo (void);
+void bar (void);
+
+int
+test (int a)
+{
+  int r;
+
+  if (r = -a)
+    foo ();
+  else
+    bar ();
+
+  return r;
+}
+
+/* { dg-final { scan-assembler-not "testl" } } */

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [x86_64 PATCH] PR tree-opt/91384: peephole2 to eliminate testl after negl.
  2022-02-28 17:36 [x86_64 PATCH] PR tree-opt/91384: peephole2 to eliminate testl after negl Roger Sayle
@ 2022-02-28 21:11 ` Uros Bizjak
  0 siblings, 0 replies; 2+ messages in thread
From: Uros Bizjak @ 2022-02-28 21:11 UTC (permalink / raw)
  To: Roger Sayle; +Cc: gcc-patches

On Mon, Feb 28, 2022 at 6:36 PM Roger Sayle <roger@nextmovesoftware.com> wrote:
>
>
> This patch is my proposed solution to PR tree-optimization/91384 which is
> a missed-optimization/code quality regression on x86_64.  The problematic
> idiom is "if (r = -a)" which is equivalent to both "r = -a; if (r != 0)"
> and alternatively "r = -a; if (a != 0)".  In this particular case, on
> x86_64, we prefer to use the condition codes from the negation, rather
> than require an explicit testl instruction.
>
> Unfortunately, combine can't help, as it doesn't attempt to merge pairs
> of instructions that share the same operand(s), only pairs/triples of
> instructions where the result of each instruction feeds the next.  But
> I doubt there's sufficient benefit to attempt this kind of "combination"
> (that wouldn't already be caught by the tree-ssa passes).
>
> Fortunately, it's relatively easy to fix this up (addressing the
> regression) during peephole2 to eliminate the unnecessary testl in:
>
>         movl    %edi, %ebx
>         negl    %ebx
>         testl   %edi, %edi
>         je      .L2
>
> Tested on x86_64-pc-linux-gnu with make bootstrap and make -k check,
> both with and without --target_board='unix{-m32\ -march=cascadelake}'
> with no new failures.  Ok for mainline?
>
>
> 2022-02-28  Roger Sayle  <roger@nextmovesoftware.com>
>
> gcc/ChangeLog
>         PR tree-optimization/91384
>         * config/i386/i386.md (peephole2): Eliminate final testl insn
>         from the sequence *movsi_internal, *negsi_1, *cmpsi_ccno_1 by
>         transforming using *negsi_2 for the negation.
>
> gcc/testsuite/ChangeLog
>         PR tree-optimization/91384
>         * gcc.target/i386/pr91384.c: New test case.

OK.

Thanks,
Uros.

>
> Thanks in advance,
> Roger
> --
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-02-28 21:12 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-28 17:36 [x86_64 PATCH] PR tree-opt/91384: peephole2 to eliminate testl after negl Roger Sayle
2022-02-28 21:11 ` Uros Bizjak

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).