On Fri, Sep 30, 2016 at 10:17 PM, Jakub Jelinek <ja...@redhat.com> wrote: > On Thu, Sep 29, 2016 at 06:21:13PM +0200, Marek Polacek wrote: >> On Tue, Sep 27, 2016 at 09:58:20PM +0200, Jakub Jelinek wrote: >> > On Tue, Sep 27, 2016 at 09:29:10PM +0200, Florian Weimer wrote: >> > > Not sure if I read this code correctly, but if we fall through from >> > > V32HImode, and we have TARGET_SSE2 set, we execute this code: >> > > >> > > tmp = "p<logic>"; >> > > ssesuffix = TARGET_AVX512VL ? "q" : ""; >> > > >> > > And not gcc_unreachable (), as is probably intended. >> > >> > It really doesn't matter. >> > The instruction uses >> > (define_mode_iterator VI12_AVX_AVX512F >> > [ (V64QI "TARGET_AVX512F") (V32QI "TARGET_AVX") V16QI >> > (V32HI "TARGET_AVX512F") (V16HI "TARGET_AVX") V8HI]) >> > iterator (and, after all, ix86_hard_regno_mode_ok should ensure the same), >> > which means V64QI/V32HI will only show up for TARGET_AVX512F, V32QI/V16HI >> > only for TARGET_AVX (which implies TARGET_SSE2), and the slightly >> > nonsensical >> > gcc_assert (TARGET_SSE2 || TARGET_AVX512VL); >> > before the switch (the || TARGET_AVX512VL is pointless, because >> > TARGET_AVX512VL implies TARGET_SSE2 as well as TARGET_AVX2). >> > So, I'd go perhaps for (untested) following patch, first diff -up, followed >> > by diff -upb: >> >> Looks good, are you going to test/commit it? Or should I? > > Here it is, bootstrapped/regtested on x86_64-linux and i686-linux, ok for > trunk? > > 2016-09-30 Jakub Jelinek <ja...@redhat.com> > > * config/i386/sse.md (<mask_codefor><code><mode>): Add FALLTHRU > comments. Simplify asserts, remove unnecessary conditions. > Formatting fixes. > (*<code><mode>3): Likewise.
OK. Thanks, Uros.