On Wed, Feb 5, 2020 at 11:05 AM Jakub Jelinek <ja...@redhat.com> wrote: > > On Tue, Feb 04, 2020 at 02:15:04PM +0100, Uros Bizjak wrote: > > On Tue, Feb 4, 2020 at 2:13 PM Jakub Jelinek <ja...@redhat.com> wrote: > > > > > > On Tue, Feb 04, 2020 at 01:38:51PM +0100, Uros Bizjak wrote: > > > > As Richard advised, let's put this safety stuff back. Usually, in > > > > i386.md, these kind of splitters are implemented as two patterns, one > > > > (define_insn_and_split) having "#", and the other (define_insn) with a > > > > real insn. My opinion is, that this separation avoids confusion as > > > > much as possible. > > > > > > Okay. So like this if it passes bootstrap/regtest then? > > > > Yes. > > > > > 2020-02-04 Jakub Jelinek <ja...@redhat.com> > > > > > > PR target/92190 > > > * config/i386/i386-features.c (ix86_add_reg_usage_to_vzeroupper): > > > Only > > > include sets and not clobbers in the vzeroupper pattern. > > > * config/i386/sse.md (*avx_vzeroupper): Require in insn condition > > > that > > > the parallel has 17 (64-bit) or 9 (32-bit) elts. > > > (*avx_vzeroupper_1): New define_insn_and_split. > > > > > > * gcc.target/i386/pr92190.c: New test. > > > > OK. > > Unfortunately, it breaks > +FAIL: gcc.target/i386/avx-2.c (internal compiler error) > +FAIL: gcc.target/i386/avx-2.c (test for excess errors) > +FAIL: gcc.target/i386/avx-vzeroupper-10.c (internal compiler error) > +FAIL: gcc.target/i386/avx-vzeroupper-10.c (test for excess errors) > +UNRESOLVED: gcc.target/i386/avx-vzeroupper-10.c scan-assembler-times > avx_vzeroupper 3 > +FAIL: gcc.target/i386/avx-vzeroupper-11.c (internal compiler error) > +FAIL: gcc.target/i386/avx-vzeroupper-11.c (test for excess errors) > +UNRESOLVED: gcc.target/i386/avx-vzeroupper-11.c scan-assembler-times > \\\\*avx_vzeroall 1 > +UNRESOLVED: gcc.target/i386/avx-vzeroupper-11.c scan-assembler-times > avx_vzeroupper 3 > +FAIL: gcc.target/i386/avx-vzeroupper-12.c (internal compiler error) > +FAIL: gcc.target/i386/avx-vzeroupper-12.c (test for excess errors) > +UNRESOLVED: gcc.target/i386/avx-vzeroupper-12.c scan-assembler-times > \\\\*avx_vzeroall 1 > +UNRESOLVED: gcc.target/i386/avx-vzeroupper-12.c scan-assembler-times > avx_vzeroupper 4 > +FAIL: gcc.target/i386/avx-vzeroupper-5.c (internal compiler error) > +FAIL: gcc.target/i386/avx-vzeroupper-5.c (test for excess errors) > +UNRESOLVED: gcc.target/i386/avx-vzeroupper-5.c scan-assembler-times > avx_vzeroupper 1 > +FAIL: gcc.target/i386/avx-vzeroupper-7.c (internal compiler error) > +FAIL: gcc.target/i386/avx-vzeroupper-7.c (test for excess errors) > +UNRESOLVED: gcc.target/i386/avx-vzeroupper-7.c scan-assembler-times > avx_vzeroupper 1 > +FAIL: gcc.target/i386/avx-vzeroupper-8.c (internal compiler error) > +FAIL: gcc.target/i386/avx-vzeroupper-8.c (test for excess errors) > +UNRESOLVED: gcc.target/i386/avx-vzeroupper-8.c scan-assembler-times > avx_vzeroupper 1 > +FAIL: gcc.target/i386/avx-vzeroupper-9.c (internal compiler error) > +FAIL: gcc.target/i386/avx-vzeroupper-9.c (test for excess errors) > +UNRESOLVED: gcc.target/i386/avx-vzeroupper-9.c scan-assembler-times > avx_vzeroupper 4 > +FAIL: gcc.target/i386/sse-14.c (internal compiler error) > +FAIL: gcc.target/i386/sse-14.c (test for excess errors) > +FAIL: gcc.target/i386/sse-22.c (internal compiler error) > +FAIL: gcc.target/i386/sse-22.c (test for excess errors) > +FAIL: gcc.target/i386/sse-22a.c (internal compiler error) > +FAIL: gcc.target/i386/sse-22a.c (test for excess errors) > > The problem is that x86 is the only target that defines HAVE_ATTR_length and > doesn't schedule any splitting pass at -O0 after pro_and_epilogue. > > So, either we go back to handling this during vzeroupper output > (unconditionally, rather than flag_ipa_ra guarded), or we need to tweak the > split* passes for x86. > > Seems there are 5 split passes, split1 is run unconditionally before reload, > split2 is run for optimize > 0 or STACK_REGS (x86) after ra but before > epilogue_completed, split3 is run before regstack for STACK_REGS and > optimize and -fno-schedule-insns2, split4 is run before sched2 if sched2 is > run and split5 is run before shorten_branches if HAVE_ATTR_length and not > STACK_REGS. > > Attached are 3 possible incremental patches for recog.c, all of them fix > all the above regressions, but haven't fully bootstrapped/regtested any of > them yet. My preference would be the last one, which for -O0 and x86 > disables split2 and enables split3, as it doesn't add any extra passes. > The first one just enables split3 for -O0 on x86, the second one enables > split5 for -O0 on x86.
Please note that in i386.md we expect that the check for "epilogue_completed" means split4 point. There are some places with: "TARGET_64BIT && ((optimize > 0 && flag_peephole2) ? epilogue_completed : reload_completed) so for flag_peephole2, we split after peephole2 pass was performed. Apparently, we already hit this problem in the past, so the check for "optimize > 0" was added to solve -O0 -fpeephole2 combo. Another one is with && TARGET_SSE_PARTIAL_REG_DEPENDENCY && epilogue_completed where we assume that allocated registers won't change anymore when breaking SSE partial register stall. I think we should just enable split4 also for -O0. This would also allow us to remove the "optimize > 0" check above and allow us to generate a bit more optimal code even with -O0 for TARGET_SSE_PARTIAL_REG_DEPENDENCY and TARGET_AVOID_FALSE_DEP_FOR_BMI. Uros. Uros. > > Jakub