From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 89461 invoked by alias); 22 May 2015 15:48:38 -0000 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 Received: (qmail 89449 invoked by uid 89); 22 May 2015 15:48:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: e06smtp11.uk.ibm.com Received: from e06smtp11.uk.ibm.com (HELO e06smtp11.uk.ibm.com) (195.75.94.107) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Fri, 22 May 2015 15:48:36 +0000 Received: from /spool/local by e06smtp11.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 22 May 2015 16:48:32 +0100 Received: from d06dlp03.portsmouth.uk.ibm.com (9.149.20.15) by e06smtp11.uk.ibm.com (192.168.101.141) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Fri, 22 May 2015 16:48:30 +0100 Received: from b06cxnps4076.portsmouth.uk.ibm.com (d06relay13.portsmouth.uk.ibm.com [9.149.109.198]) by d06dlp03.portsmouth.uk.ibm.com (Postfix) with ESMTP id 802701B0805F for ; Fri, 22 May 2015 16:49:19 +0100 (BST) Received: from d06av06.portsmouth.uk.ibm.com (d06av06.portsmouth.uk.ibm.com [9.149.37.217]) by b06cxnps4076.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t4MFmTd55112220 for ; Fri, 22 May 2015 15:48:29 GMT Received: from d06av06.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av06.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t4MAgGmd029535 for ; Fri, 22 May 2015 06:42:16 -0400 Received: from [9.164.171.240] (icon-9-164-171-240.megacenter.de.ibm.com [9.164.171.240]) by d06av06.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id t4MAgFfK029520; Fri, 22 May 2015 06:42:15 -0400 Message-ID: <555F4FCC.5020501@linux.vnet.ibm.com> Date: Fri, 22 May 2015 16:14:00 -0000 From: Andreas Krebbel User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: "gcc-patches@gcc.gnu.org" , richard.sandiford@arm.com Subject: Re: Mostly rewrite genrecog References: <87egn5yis1.fsf@e105548-lin.cambridge.arm.com> <5556DF07.6020000@linux.vnet.ibm.com> <87bnhix61c.fsf@e105548-lin.cambridge.arm.com> In-Reply-To: <87bnhix61c.fsf@e105548-lin.cambridge.arm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15052215-0041-0000-0000-0000047D29F9 X-IsSubscribed: yes X-SW-Source: 2015-05/txt/msg02146.txt.bz2 On 05/17/2015 11:12 PM, Richard Sandiford wrote: > Andreas Krebbel writes: >> Hi Richard, >> >> I see regressions with the current IBM z13 vector patchset which appear to be related to the new >> genrecog. >> >> The following two insn definitions only differ in the mode and predicate of the shift count operand. >> >> (define_insn "lshr3" >> [(set (match_operand:VI 0 "register_operand" "=v") >> (lshiftrt:VI (match_operand:VI 1 "register_operand" "v") >> (match_operand:SI 2 "shift_count_or_setmem_operand" "Y")))] >> "TARGET_VX" >> "vesrl\t%v0,%v1,%Y2" >> [(set_attr "op_type" "VRS")]) >> >> (define_insn "vlshr3" >> [(set (match_operand:VI 0 "register_operand" "=v") >> (lshiftrt:VI (match_operand:VI 1 "register_operand" "v") >> (match_operand:VI 2 "register_operand" "v")))] >> "TARGET_VX" >> "vesrlv\t%v0,%v1,%v2" >> [(set_attr "op_type" "VRR")]) >> >> >> However, the insn-recog.c code only seem to check the predicate. This is a problem since >> shift_count_or_setmem_operand does not check the mode. > > Yeah, it's a bug if a "non-special" predicate doesn't check the mode. > Even old genreog relied on that: > > /* After factoring, try to simplify the tests on any one node. > Tests that are useful for switch statements are recognizable > by having only a single test on a node -- we'll be manipulating > nodes with multiple tests: > > If we have mode tests or code tests that are redundant with > predicates, remove them. */ > > although it sounds like the old optimisation didn't trigger for your case. > > genpreds.c:mark_mode_tests is supposed to add these tests automatically > if needed. I suppose it isn't doing so here because the predicate > accepts const_int and because of: > > /* Given an RTL expression EXP, find all subexpressions which we may > assume to perform mode tests. Normal MATCH_OPERAND does; > MATCH_CODE does if it applies to the whole expression and accepts > CONST_INT or CONST_DOUBLE; and we have to assume that MATCH_TEST > does not. [...] > */ > static void > mark_mode_tests (rtx exp) > { > switch (GET_CODE (exp)) > { > [...] > case MATCH_CODE: > if (XSTR (exp, 1)[0] != '\0' > || (!strstr (XSTR (exp, 0), "const_int") > && !strstr (XSTR (exp, 0), "const_double"))) > NO_MODE_TEST (exp) = 1; > break; > > The code matches the comment, but it doesn't look right. Perhaps it > was supposed to mean match_codes that _only_ contain const_int and > const_double? Knowing that the rtx is one of those codes guarantees > that the mode is VOIDmode, but a match_code that includes other rtxes > as well doesn't itself test the mode of those rtxes. > > Even then, a predicate that matches const_ints and is passed SImode > mustn't accept const_ints outside the SImode range. I suppose we > just rely on all predicates to perform some kind of range check. > (The standard ones do of course.) > > As a quick workaround, try replacing the case above with: > > case MATCH_CODE: > if (XSTR (exp, 1)[0] != '\0') > NO_MODE_TEST (exp) = 1; > break; > > I'll try to come up with a better fix in the meantime. Thanks for looking into this! I've applied a patch adding a mode check to shift_count_or_setmem_operand which as expected fixes the issue for me. -Andreas-