From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 107233 invoked by alias); 3 Jan 2019 14:36:46 -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 107207 invoked by uid 89); 3 Jan 2019 14:36:46 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=games, H*RU:209.85.214.196, Hx-spam-relays-external:209.85.214.196, playing X-HELO: mail-pl1-f196.google.com Received: from mail-pl1-f196.google.com (HELO mail-pl1-f196.google.com) (209.85.214.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 03 Jan 2019 14:36:44 +0000 Received: by mail-pl1-f196.google.com with SMTP id w4so16009758plz.1 for ; Thu, 03 Jan 2019 06:36:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=f3O7QUAdFHB6ACg9y/o2Bnwy4u9Arxbe8Z+Zih1DLXE=; b=SgbzfMY9RDqkXh3jjwwNdWO9T4y2uudSrYsjj9bPsSgh/QMw+DHaC05as8OpMKQF53 RSIpntsfoqwAdCE4Agu3kdkGouhgwrXu/pv973X7eahNR2nRmSJfVBXZ84tq4nRwR5Te KresixoGmBO5cP4ABUNoMdaZwGvziqKgj2hhLHxgi8KTxIlng98Qb6ftSBloIIlFsQx1 I0c1ybHZd7QJ/BhcvQMX3GqIxqMqGQv5YXrNy5H9jpyTaoAh+JHcjpIQpC8FhBhXG6AT RNA33yMVjiKrE1NBZgr3A4+ib8pvA58MjKrQxaOQ8etClySaBYeLyU5dTRya+FUQCzr+ w77Q== Return-Path: Received: from bubble.grove.modra.org ([58.175.241.133]) by smtp.gmail.com with ESMTPSA id v89sm84158203pfk.12.2019.01.03.06.36.41 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 03 Jan 2019 06:36:41 -0800 (PST) Received: by bubble.grove.modra.org (Postfix, from userid 1000) id 3FC8580D26; Fri, 4 Jan 2019 01:06:38 +1030 (ACDT) Date: Thu, 03 Jan 2019 14:36:00 -0000 From: Alan Modra To: gcc-patches@gcc.gnu.org, richard.sandiford@arm.com Subject: Re: [PATCH] genattrtab bit-rot, and if_then_else in values Message-ID: <20190103143637.GA3170@bubble.grove.modra.org> References: <20190103092154.GZ30978@bubble.grove.modra.org> <87a7khsyjz.fsf@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87a7khsyjz.fsf@arm.com> User-Agent: Mutt/1.9.4 (2018-02-28) X-IsSubscribed: yes X-SW-Source: 2019-01/txt/msg00108.txt.bz2 On Thu, Jan 03, 2019 at 12:41:52PM +0000, Richard Sandiford wrote: > Alan Modra writes: > > + case PLUS: > > + current_or = or_attr_value (XEXP (exp, 0)); > > + if (current_or != -1) > > + { > > + int n = current_or; > > + current_or = or_attr_value (XEXP (exp, 1)); > > + if (current_or != -1) > > + current_or += n; > > + } > > + break; > > This doesn't look right. Doing the same for IOR and |= would be OK > in principle, but write_attr_value doesn't handle IOR yet. >From what I can see, or_attr_value is used to find the largest power of two that divides all attr length values for a given insn. So what should be done for PLUS in an attr length value expression? I can't simply ignore it as then or_attr_value will return -1 and we'll calculate length_unit_log as 0 on powerpc when it ought to be 2. That might matter some time in the future. If the operands of PLUS are assumed to be insn lengths (reasonable, since you're likely to use PLUS to sum the machine insn lengths of a pattern that conditionally emits multiple machine insns), then it might be better to replace "current_or += n" above with "current_or |= n". That would be correct for a length expression of (plus (if_then_else (condA) (const_int 4) (const_int 0)) (if_then_else (condB) (const_int 8) (const_int 4))) where any answer from or_attr_value that has 3 lsbs of 100b is sufficently correct. If I keep "+=" then we'd get the wrong answer in this particular example. However "|=" is wrong if someone is playing games and writes a silly expression like (plus (const_int 1) (const_int 3)) since the length is always 4. > OK with the above dropped, thanks. Maybe this instead? case PLUS: current_or = or_attr_value (XEXP (exp, 0)); current_or |= or_attr_value (XEXP (exp, 1)); break; > Richard > > PS. current write_attr_value doesn't seem to handle operator precedence > properly, but that's orthogonal. I hadn't noticed that, but yes that is another problem. I also didn't implement MINUS, MULT, DIV and MOD in max_attr_value, min_attr_value and or_attr_value even though write_attr_value handles those cases. -- Alan Modra Australia Development Lab, IBM