From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 90997 invoked by alias); 1 Feb 2016 16:58:59 -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 90983 invoked by uid 89); 1 Feb 2016 16:58:58 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=Especially, recording, doubled, safer X-HELO: e06smtp06.uk.ibm.com Received: from e06smtp06.uk.ibm.com (HELO e06smtp06.uk.ibm.com) (195.75.94.102) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Mon, 01 Feb 2016 16:58:57 +0000 Received: from localhost by e06smtp06.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 1 Feb 2016 16:58:53 -0000 Received: from d06dlp02.portsmouth.uk.ibm.com (9.149.20.14) by e06smtp06.uk.ibm.com (192.168.101.136) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Mon, 1 Feb 2016 16:58:51 -0000 X-IBM-Helo: d06dlp02.portsmouth.uk.ibm.com X-IBM-MailFrom: uweigand@de.ibm.com X-IBM-RcptTo: gcc-patches@gcc.gnu.org Received: from b06cxnps4074.portsmouth.uk.ibm.com (d06relay11.portsmouth.uk.ibm.com [9.149.109.196]) by d06dlp02.portsmouth.uk.ibm.com (Postfix) with ESMTP id 7EFA62190056 for ; Mon, 1 Feb 2016 16:58:37 +0000 (GMT) Received: from d06av01.portsmouth.uk.ibm.com (d06av01.portsmouth.uk.ibm.com [9.149.37.212]) by b06cxnps4074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u11Gwo824129114 for ; Mon, 1 Feb 2016 16:58:50 GMT Received: from d06av01.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av01.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u11GwocU002544 for ; Mon, 1 Feb 2016 09:58:50 -0700 Received: from oc7340732750.ibm.com (dyn-9-152-213-148.boeblingen.de.ibm.com [9.152.213.148]) by d06av01.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id u11Gwo0O002539; Mon, 1 Feb 2016 09:58:50 -0700 Received: by oc7340732750.ibm.com (Postfix, from userid 500) id 3871A529E; Mon, 1 Feb 2016 17:58:50 +0100 (CET) Subject: Re: [PATCH 3/9] S/390: Get rid of Y constraint in rotate patterns. To: krebbel@linux.vnet.ibm.com (Andreas Krebbel) Date: Mon, 01 Feb 2016 16:58:00 -0000 From: "Ulrich Weigand" Cc: gcc-patches@gcc.gnu.org In-Reply-To: <56AF8BFF.9000602@linux.vnet.ibm.com> from "Andreas Krebbel" at Feb 01, 2016 05:46:55 PM MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-Id: <20160201165850.3871A529E@oc7340732750.ibm.com> X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16020116-0025-0000-0000-000005B6D758 X-SW-Source: 2016-02/txt/msg00056.txt.bz2 Andreas Krebbel wrote: > On 02/01/2016 02:30 PM, Ulrich Weigand wrote: > > This seems to add just a single alternative. Is that OK if it gets > > substituted into a pattern that used multiple alternatives otherwise? > Yes. This is supposed to work. The new constraint will be duplicated until it matches the number > of alternatives in the original insn. OK, that makes sense. But now I'm wondering about this bit of the ashiftrt patch: +; FIXME: The number of alternatives is doubled here to match the fix +; number of 4 in the subst pattern for the (clobber (match_scratch... +; The right fix should be to support match_scratch in the output +; pattern of a define_subst. +(define_subst "cconly_subst" + [(set (match_operand:DSI 0 "" "") + (match_operand:DSI 1 "" "")) + (clobber (reg:CC CC_REGNUM))] + "s390_match_ccmode(insn, CCSmode)" + [(set (reg CC_REGNUM) + (compare (match_dup 1) (const_int 0))) + (clobber (match_scratch:DSI 0 "=d,d,d,d"))]) I guess this is where the number 4 comes from? But if the alternatives are duplicated, shouldn't it then work to simply use: (clobber (match_scratch:DSI 0 "=d"))]) > >> +; In the subst pattern we have to disable the alternative where op2 is > >> +; already a constant. This attribute is supposed to be used in the > >> +; "disabled" insn attribute to achieve this. > >> +(define_subst_attr "addr_style_op_disable" "addr_style_op_subst" "0" "1") > > > > Is this necessary given that the subst adds a REG_P (operands[2]) > > condition? > I wasn't sure about this. How does reload/lra behave when the constraints allow more than the insn > condition? I expected that reload might remember that one of the alternatives might take a constant > as well and then tries as part of reload inheritance and such things to actually exploit this?! Just > disabling the alternative in order to prevent reload from recording stuff for that alternative > appeared safer to me. > > On the other hand it is not ideal to have an operands[x] check in the insn condition in the first > place. Especially when using define_substs where the operands get renumbered this might add hard to > find bugs. Perhaps I should try replacing the insn predicate with a subst_attr instead? Is having a > constraint which allows more than a predicate any better than having the same with the insn condition? Hmm. In theory reload should always respect both constraints and predicates. But I guess it can't hurt to disable the alternative just to be safe; it is indeed an uncommon case to have the constraint accept more than the predicate. (Also a PLUS with two CONST_INT operands is not canonical RTL anyway.) Bye, Ulrich -- Dr. Ulrich Weigand GNU/Linux compilers and toolchain Ulrich.Weigand@de.ibm.com