public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Kyrill  Tkachov <kyrylo.tkachov@foss.arm.com>
To: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Cc: Uros Bizjak <ubizjak@gmail.com>
Subject: [PATCH][i386][3/3] PR target/84164: Make *cmpqi_ext_<n> patterns accept more zero_extract modes
Date: Thu, 08 Feb 2018 17:11:00 -0000	[thread overview]
Message-ID: <5A7C84A7.9090500@foss.arm.com> (raw)

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

Hi all,

This patch fixes some fallout in the i386 testsuite that occurs after the simplification in patch [1/3] [1].
The gcc.target/i386/extract-2.c FAILs because it expects to match:
(set (reg:CC 17 flags)
     (compare:CC (subreg:QI (zero_extract:SI (reg:HI 98)
                 (const_int 8 [0x8])
                 (const_int 8 [0x8])) 0)
         (const_int 4 [0x4])))

which is the *cmpqi_ext_2 pattern in i386.md but with the new simplification the combine/simplify-rtx
machinery produces:
(set (reg:CC 17 flags)
     (compare:CC (subreg:QI (zero_extract:HI (reg:HI 98)
                 (const_int 8 [0x8])
                 (const_int 8 [0x8])) 0)
         (const_int 4 [0x4])))

Notice that the zero_extract now has HImode like the register source rather than SImode.
The existing *cmpqi_ext_<n> patterns however explicitly demand an SImode on the zero_extract.
I'm not overly familiar with the i386 port but I think that's too restrictive.
The RTL documentation says:
For (zero_extract:m loc size pos) "The mode m is the same as the mode that would be used for loc if it were a register."
I'm not sure if that means that the mode of the zero_extract and the source register must always match (as is the
case after patch [1/3]) but in any case it shouldn't matter semantically since we're taking a QImode subreg of the whole
thing anyway.

So the proposed solution in this patch is to allow HI, SI and DImode zero_extracts in these patterns as these are the
modes that the ext_register_operand predicate accepts, so that the patterns can match the new form above.

With this patch the aforementioned test passes again and bootstrap and testing on x86_64-unknown-linux-gnu shows
no regressions.

Is this ok for trunk if the first patch is accepted?

Thanks,
Kyrill

[1] https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00443.html

2018-02-07  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR target/84164
     * config/i386/i386.md (*cmpqi_ext_1): Rename to...
     (*cmpqi<mode>_ext_1): ... This.  Use SWI248 mode iterator
     for zero_extract.
     (*cmpqi_ext_2): Rename to...
     (*cmpqi<mode>_ext_2): ... This.  Use SWI248 mode iterator
     for zero_extract.
     (*cmpqi_ext_3): Rename to...
     (*cmpqi<mode>_ext_3): ... This.  Use SWI248 mode iterator
     for zero_extract.
     (*cmpqi_ext_4): Rename to...
     (*cmpqi<mode>_ext_4): ... This.  Use SWI248 mode iterator
     for zero_extract.

[-- Attachment #2: i386-modes.patch --]
[-- Type: text/x-patch, Size: 1991 bytes --]

diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index a4832bf696f321e8ee5aad71fa946ca198d9d689..ced9a3e823ae6c4586be510a782d354f4d364daa 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -1328,12 +1328,12 @@ (define_insn "*cmp<mode>_minus_1"
   [(set_attr "type" "icmp")
    (set_attr "mode" "<MODE>")])
 
-(define_insn "*cmpqi_ext_1"
+(define_insn "*cmpqi<mode>_ext_1"
   [(set (reg FLAGS_REG)
 	(compare
 	  (match_operand:QI 0 "nonimmediate_operand" "QBc,m")
 	  (subreg:QI
-	    (zero_extract:SI
+	    (zero_extract:SWI248
 	      (match_operand 1 "ext_register_operand" "Q,Q")
 	      (const_int 8)
 	      (const_int 8)) 0)))]
@@ -1343,11 +1343,11 @@ (define_insn "*cmpqi_ext_1"
    (set_attr "type" "icmp")
    (set_attr "mode" "QI")])
 
-(define_insn "*cmpqi_ext_2"
+(define_insn "*cmpqi<mode>_ext_2"
   [(set (reg FLAGS_REG)
 	(compare
 	  (subreg:QI
-	    (zero_extract:SI
+	    (zero_extract:SWI248
 	      (match_operand 0 "ext_register_operand" "Q")
 	      (const_int 8)
 	      (const_int 8)) 0)
@@ -1368,11 +1368,11 @@ (define_expand "cmpqi_ext_3"
 	      (const_int 8)) 0)
 	  (match_operand:QI 1 "const_int_operand")))])
 
-(define_insn "*cmpqi_ext_3"
+(define_insn "*cmpqi<mode>_ext_3"
   [(set (reg FLAGS_REG)
 	(compare
 	  (subreg:QI
-	    (zero_extract:SI
+	    (zero_extract:SWI248
 	      (match_operand 0 "ext_register_operand" "Q,Q")
 	      (const_int 8)
 	      (const_int 8)) 0)
@@ -1383,16 +1383,16 @@ (define_insn "*cmpqi_ext_3"
    (set_attr "type" "icmp")
    (set_attr "mode" "QI")])
 
-(define_insn "*cmpqi_ext_4"
+(define_insn "*cmpqi<mode>_ext_4"
   [(set (reg FLAGS_REG)
 	(compare
 	  (subreg:QI
-	    (zero_extract:SI
+	    (zero_extract:SWI248
 	      (match_operand 0 "ext_register_operand" "Q")
 	      (const_int 8)
 	      (const_int 8)) 0)
 	  (subreg:QI
-	    (zero_extract:SI
+	    (zero_extract:SWI248
 	      (match_operand 1 "ext_register_operand" "Q")
 	      (const_int 8)
 	      (const_int 8)) 0)))]

             reply	other threads:[~2018-02-08 17:11 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-08 17:11 Kyrill Tkachov [this message]
2018-02-08 22:54 ` Uros Bizjak
2018-02-09 14:50   ` Kyrill Tkachov
2018-02-13 16:46     ` Jeff Law
2018-02-14 18:04       ` Kyrill Tkachov
2018-02-15  7:25         ` Uros Bizjak
2018-02-16 17:38           ` Kyrill Tkachov

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=5A7C84A7.9090500@foss.arm.com \
    --to=kyrylo.tkachov@foss.arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=ubizjak@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).