[PATCH][i386][3/3] PR target/84164: Make *cmpqi_ext_<n> patterns accept more zero_extract modes

Uros Bizjak ubizjak@gmail.com
Thu Feb 8 22:54:00 GMT 2018


On Thu, Feb 8, 2018 at 6:11 PM, Kyrill  Tkachov
<kyrylo.tkachov@foss.arm.com> wrote:
> 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?

Huh, there are many other zero-extract patterns besides cmpqi_ext_*
with QImode subreg of SImode zero_extract in i386.md, used to access
high QImode register of HImode pair. A quick grep shows these that
have _ext_ in their name:

(define_insn "*cmpqi_ext_1"
(define_insn "*cmpqi_ext_2"
(define_expand "cmpqi_ext_3"
(define_insn "*cmpqi_ext_3"
(define_insn "*cmpqi_ext_4"
(define_insn "addqi_ext_1"
(define_insn "*addqi_ext_2"
(define_expand "testqi_ext_1_ccno"
(define_insn "*testqi_ext_1"
(define_insn "*testqi_ext_2"
(define_insn_and_split "*testqi_ext_3"
(define_insn "andqi_ext_1"
(define_insn "*andqi_ext_1_cc"
(define_insn "*andqi_ext_2"
(define_insn "*<code>qi_ext_1"
(define_insn "*<code>qi_ext_2"
(define_expand "xorqi_ext_1_cc"
(define_insn "*xorqi_ext_1_cc"

There are also relevant splitters and peephole2 patterns.

IIRC, SImode zero_extract was enough to catch all high-register uses.
There will be a pattern explosion if we want to handle all other
integer modes here. However, I'm not a RTL expert, so someone will
have to say what is the correct RTX form here.

Uros.



More information about the Gcc-patches mailing list