From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9394 invoked by alias); 30 Aug 2011 08:50:30 -0000 Received: (qmail 9376 invoked by uid 22791); 30 Aug 2011 08:50:28 -0000 X-SWARE-Spam-Status: No, hits=-3.5 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,TW_ZJ X-Spam-Check-By: sourceware.org Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 30 Aug 2011 08:50:12 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.221.2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id 42E838BB22; Tue, 30 Aug 2011 10:50:10 +0200 (CEST) Date: Tue, 30 Aug 2011 09:19:00 -0000 From: Richard Guenther To: Uros Bizjak Cc: gcc-patches@gcc.gnu.org, uros@gcc.gnu.org, rth@redhat.com, artyom.shinkaroff@gmail.com Subject: Re: [PATCH] Change vcond to vcond In-Reply-To: Message-ID: References: User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 Content-Type: MULTIPART/MIXED; BOUNDARY="8323584-2108993114-1314694210=:2130" Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2011-08/txt/msg02417.txt.bz2 This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --8323584-2108993114-1314694210=:2130 Content-Type: TEXT/PLAIN; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT Content-length: 6218 On Tue, 30 Aug 2011, Uros Bizjak wrote: > On Mon, Aug 29, 2011 at 9:44 PM, Richard Guenther wrote: > > >> > This patch makes a conversion optab from the direct optabs vcond > >> > and vcondu.  This allows to specify different modes for the > >> > actual comparison and the value that is selected. > >> > > >> > All targets but i386 are trivially converted by > >> > s/vcond/vcond/.  The i386 port is enhanced > >> > to support a OP b ? c : d as ({ mask = a OP b; (c & mask) | (d & ~mask); > >> > }), constraining it to what the middle-end constrained itself to > >> > (matching number of vector elements in the comparison operands with > >> > the result vector types) would explode patterns too much. > >> > Thus, only a subset of mode combinations will be excercised > >> > (but none at the moment - a followup will fix the vectorizer, > >> > and generic vectors from the C extensions have a patch pending). > >> > > >> > Bootstrapped on x86_64-unknown-linux-gnu, tests are currently > >> > running for {,-m32}. > >> > > >> > Ok if that succeeds? > >> > > >> > Thanks, > >> > Richard. > >> > > >> > 2011-08-29  Richard Guenther   > >> > > >> >        * genopinit.c (optabs): Turn vcond{,u}_optab into a conversion > >> >        optab with two modes. > >> >        * optabs.h (enum convert_optab_index): Add COI_vcond, COI_vcondu. > >> >        (enum direct_optab_index): Remove DOI_vcond, DOI_vcondu. > >> >        (vcond_optab): Adjust. > >> >        (vcondu_optab): Likewise. > >> >        (expand_vec_cond_expr_p): Adjust prototype. > >> >        * optabs.c (get_vcond_icode): Adjust. > >> >        (expand_vec_cond_expr_p): Likewise. > >> >        (expand_vec_cond_expr): Likewise. > >> >        * tree-vect-stmt.c (vectorizable_condition): Adjust. > >> > > >> >        * config/i386/sse.md (vcond): Split to > >> >        vcond, vcond, > >> >        vcond and > >> >        vcondu. > >> >        (vcondv2di): Change to vcondv2di. > >> >        (vconduv2di): Likewise. > >> >        * config/arm/neon.md (vcond): Change to vcond*. > >> >        (vcondu): Likewise. > >> >        * config/ia64/vect.md (vcond): Likewise. > >> >        (vcondu): Likewise. > >> >        (vcondv2sf): Likewise. > >> >        * config/mips/mips-ps-3d.md (vcondv2sf): Likewise. > >> >        * config/rs6000/paired.md (vcondv2sf): Likewise. > >> >        * config/rs6000/vector.md (vcond): Likewise. > >> >        (vcondu): Likewise. > >> >        * config/spu/spu.md (vcond): Likewise. > >> >        (vcondu): Likewise. > >> > >> Do we really want to introduce stuff like: > >> > >> ! (define_expand "vcond" > >> > >> You are in fact introducing 6x2 = 12 patterns, many of them (i.e. > >> v16qiv2df combination) invalid. > > > > Well, in principle they are not "invalid" they would be a short-hand - > > the example of (subreg:V16QI (vcond:V2DI (... )). > > > >> I'd prefer a pattern with mode-less operands 4 and 5, rejected in insn > >> constraints for invalid combinations: > > > > Hm, ok - that was the first variant I tried (well, but with modeless > > operands 1 and 2, to keep 4 and 5 selcting int vs. fp compare).  But > > modeless operands get you that annoying warning from the gen* programs. > > Only for define_insn, if your c_test does not include string "operands". > > > How'd you ask if a pattern is available for vcondv4si with v4sf > > operands 4 and 5?  The vectorizer would need to be able to ask this > > question. > > Maybe with something along the lines of: > > (define_expand "vcond" > [(set (match_operand:VI124_128 0 "register_operand" "") > (if_then_else:VI124_128 > (match_operator 3 "" > [(match_operand 4 "nonimmediate_operand" "") > (match_operand 5 "nonimmediate_operand" "")]) > (match_operand:VI124_128 1 "general_operand" "") > (match_operand:VI124_128 2 "general_operand" "")))] > "TARGET_SSE2" > { > if (GET_MODE (operands[4]) != GET_MODE (operands[5]) > || (GET_MODE_NUNITS (GET_MODE (operands[4])) > != GET_MODE_NUNITS (GET_MODE (operands[0])))) > FAIL; > > bool ok = ix86_expand_int_vcond (operands); > gcc_assert (ok); > DONE; > }) > > This means that vcond pattern is allowed to FAIL, so when vectorizer > tentatively tries to expand the pattern, FAIL signalizes that operand > modes are not supported. Hmm. But then I'd have to try emit an insn, right? Currently the vectorizer simply looks for an optab handler ... the operands are not readily available (but their mode is known). So I'd create some fake regs, setup operands and call GEN_FCN on it? If it succeds I'd have to delete emitted insns, etc. Or I could add a target hook ... That shifts the ugliness towards the users, for just saving a few redundant patterns? > > In the end putting both mask generation and apply into one > > instruction pattern causes all this issues - but it helps > > not exposing the mask representation of the HW. > > No, but we should be sure that the widths are the same... Yes. We'd guarantee that we'd never try to expand something where that doesn't hold. I just saw PR29269 - vcond is not documented ... I'd document it for the two-mode variant as @cindex @code{vcond@var{m}@var{n}} instruction pattern @item @samp{vcond@var{m}@var{n}} Output a conditional vector move. Operand 0 is the destination to receive a combination of operand 1 and operand 2, which are of mode @var{m}, dependent on the outcome of the predicate in operand 3 which is a vector comparison with operands of mode @var{n} in operands 4 and 5. The modes @var{m} and @var{n} should have the same size. Operand 0 will be set to the value @var{op1} & @var{msk} | @var{op2} & ~@var{msk} where @var{msk} is computed by element-wise evaluation of the vector comparison with a truth value of all-ones and a false value of all-zeros. not constraining the modes further than requiring the same size. The two patches tested ok on x86_64-unknown-linux-gnu with {,-m32}. Richard. > Uros. --8323584-2108993114-1314694210=:2130--