From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7803 invoked by alias); 17 May 2015 21:12:23 -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 7784 invoked by uid 89); 17 May 2015 21:12:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.8 required=5.0 tests=AWL,BAYES_20,SPF_PASS autolearn=ham version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (146.101.78.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 17 May 2015 21:12:20 +0000 Received: from cam-owa2.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by uk-mta-25.uk.mimecast.lan; Sun, 17 May 2015 22:12:17 +0100 Received: from localhost ([10.1.2.79]) by cam-owa2.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Sun, 17 May 2015 22:12:15 +0100 From: Richard Sandiford To: Andreas Krebbel Mail-Followup-To: Andreas Krebbel ,"gcc-patches\@gcc.gnu.org" , richard.sandiford@arm.com Cc: "gcc-patches\@gcc.gnu.org" Subject: Re: Mostly rewrite genrecog References: <87egn5yis1.fsf@e105548-lin.cambridge.arm.com> <5556DF07.6020000@linux.vnet.ibm.com> Date: Sun, 17 May 2015 22:05:00 -0000 In-Reply-To: <5556DF07.6020000@linux.vnet.ibm.com> (Andreas Krebbel's message of "Sat, 16 May 2015 07:09:11 +0100") Message-ID: <87bnhix61c.fsf@e105548-lin.cambridge.arm.com> User-Agent: Gnus/5.130012 (Ma Gnus v0.12) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-MC-Unique: Xwn5tlx5QfWX9vjC43wlfg-1 Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable X-SW-Source: 2015-05/txt/msg01517.txt.bz2 Andreas Krebbel writes: > Hi Richard, > > I see regressions with the current IBM z13 vector patchset which appear t= o 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" "= =3Dv") > (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" "=3Dv") > (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] !=3D '\0' || (!strstr (XSTR (exp, 0), "const_int") && !strstr (XSTR (exp, 0), "const_double"))) NO_MODE_TEST (exp) =3D 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] !=3D '\0') NO_MODE_TEST (exp) =3D 1; break; I'll try to come up with a better fix in the meantime. Thanks, Richard