On Thu, Feb 8, 2018 at 6:11 PM, Kyrill Tkachov <kyrylo.tkac...@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.