From patchwork Fri Nov 13 19:00:29 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Andrew MacLeod X-Patchwork-Id: 1400044 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=gcc.gnu.org Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=dQTPs4ta; dkim-atps=neutral Received: from sourceware.org (unknown [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4CXnqL5KNwz9sTK for ; Sat, 14 Nov 2020 06:00:43 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 29488389803B; Fri, 13 Nov 2020 19:00:40 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 29488389803B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1605294040; bh=3l1z9Ubddr+wBq7H60dHCGyMqehpw6MCWoemQnRuRrQ=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=dQTPs4ta+jPR8R1ltl21SPgNVNCqVz1LNCDs4tDxwF2vaKV76scNsQRVrwZ0wl1Ac MlrbfWGGtp7usNYjESdIBOiNpN855/ShfWh/ed4WTdjWrt4I2vN36PuCkwFeNkFQ7a ypLub6GysIqJk+ZFw4bAiM6XWskjUhSSOTqTcZz4= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id E212D385482C for ; Fri, 13 Nov 2020 19:00:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E212D385482C Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-262-1qNUlzk-MHyvboNTSPdsRw-1; Fri, 13 Nov 2020 14:00:35 -0500 X-MC-Unique: 1qNUlzk-MHyvboNTSPdsRw-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 19685805EFD; Fri, 13 Nov 2020 19:00:34 +0000 (UTC) Received: from [10.10.119.201] (ovpn-119-201.rdu2.redhat.com [10.10.119.201]) by smtp.corp.redhat.com (Postfix) with ESMTP id 893546EF6E; Fri, 13 Nov 2020 19:00:31 +0000 (UTC) Subject: [PATCH] Re: Fix gimple_expr_code? To: Richard Biener References: <7dc11892-b595-68b2-7b28-5b16eec63941@redhat.com> <5975d49f-6ad3-df1e-838f-b7a7443a5dbb@redhat.com> Message-ID: Date: Fri, 13 Nov 2020 14:00:29 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00, BODY_8BITS, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Andrew MacLeod via Gcc-patches From: Andrew MacLeod Reply-To: Andrew MacLeod Cc: Andrew MacLeod via Gcc-patches Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" On 11/13/20 4:05 AM, Richard Biener wrote: > On Thu, Nov 12, 2020 at 10:12 PM Andrew MacLeod > wrote: > > On 11/12/20 3:53 PM, Richard Biener wrote: > > On November 12, 2020 9:43:52 PM GMT+01:00, Andrew MacLeod via > Gcc-patches > wrote: > >> So I spent some time tracking down a ranger issue, and in the > end, it > >> boiled down to the range-op handler not being picked up properly. > >> > >> The handler is picked up by: > >> > >>    if ((gimple_code (s) == GIMPLE_ASSIGN) || (gimple_code (s) == > >> GIMPLE_COND)) > >>      return range_op_handler (gimple_expr_code (s), > gimple_expr_type > >> (s)); > > IMHO this should use more specific functions. Gimple_expr_code > should go away similar to gimple_expr_type. > > gimple_expr_type is quite pervasive.. and each consumer is going > to have > to roll their own version of it.  Why do we want to get rid of it? > > If we are trying to save a few bytes by storing the information in > different places, then we're going to need some sort of accessing > function like that > > > >> where it is indexing the table with the gimple_expr_code.. > >> the stmt being processed was for a pointer assignment, > >>    _5 = _33 > >> and it was coming back with a gimple_expr_code of VAR_DECL > instead of > >> an SSA_NAME... which confused me greatly. > >> > >> > >> gimple_expr_code (const gimple *stmt) > >> { > >>    enum gimple_code code = gimple_code (stmt); > >>    if (code == GIMPLE_ASSIGN || code == GIMPLE_COND) > >>      return (enum tree_code) stmt->subcode; > >> > >> A little more digging shows this: > >> > >> static inline enum tree_code > >> gimple_assign_rhs_code (const gassign *gs) > >> { > >>    enum tree_code code = (enum tree_code) gs->subcode; > >>    /* While we initially set subcode to the TREE_CODE of the > rhs for > >>       GIMPLE_SINGLE_RHS assigns we do not update that subcode > to stay > >>       in sync when we rewrite stmts into SSA form or do SSA > >> propagations.  */ > >>    if (get_gimple_rhs_class (code) == GIMPLE_SINGLE_RHS) > >>      code = TREE_CODE (gs->op[1]); > >> > >>    return code; > >> } > >> > >> Fascinating comment. > > ... 😬 > > > >> But it means that gimple_expr_code() isn't returning the > correct result > >> > >> for GIMPLE_SINGLE_RHS.... > > It depends. A SSA name isn't an expression code either. As said, > the generic gimple_expr_code should be used with extreme care. > > what is an expression code?  It seems like its just a tree_code > representing what is on the RHS?    Im not sure I understand why one > needs to be careful with it.  It only applies to COND, ASSIGN and > CALL. > and its current right for everything except GIMPLE_SINGLE_RHS? > > If we dont fix gimple_expr_code, then Im basically going to be > reimplementing it myself... which seems kind of pointless. > > > Well sure we can fix it.  Your patch looks OK but can be optimized like > >   if (gassign *ass = dyn_cast (stmt)) >     return gimple_assign_rhs_code (stmt); > > note it looks odd that we use this for gimple_assign but > directly access subcode for GIMPLE_COND instead > of returning gimple_cond_code () (again, operate on > gcond to avoid an extra check). > > Thanks, > Richard. > > Andrew > > > And with a little bit of const-ness...  I adjusted gimple_range_handler to use gassing and gcond as well. Bootstrapped on x86_64-pc-linux-gnu, no regressions. pushed. Andrew commit fcbb6018abaf04d30e2cf6fff2eb35115412cdd5 Author: Andrew MacLeod Date: Fri Nov 13 13:56:01 2020 -0500 Re: Fix gimple_expr_code? have gimple_expr_code return the correct code for GIMPLE_ASSIGN. use gassign and gcond in gimple_range_handler. * gimple-range.h (gimple_range_handler): Cast to gimple stmt kinds before asking for code and type. * gimple.h (gimple_expr_code): Call gassign and gcond routines to get their expr_code. diff --git a/gcc/gimple-range.h b/gcc/gimple-range.h index dde41e9e743..92bb5305c18 100644 --- a/gcc/gimple-range.h +++ b/gcc/gimple-range.h @@ -97,12 +97,12 @@ extern bool gimple_range_calc_op2 (irange &r, const gimple *s, static inline range_operator * gimple_range_handler (const gimple *s) { - if (gimple_code (s) == GIMPLE_ASSIGN) - return range_op_handler (gimple_assign_rhs_code (s), - TREE_TYPE (gimple_assign_lhs (s))); - if (gimple_code (s) == GIMPLE_COND) - return range_op_handler (gimple_cond_code (s), - TREE_TYPE (gimple_cond_lhs (s))); + if (const gassign *ass = dyn_cast (s)) + return range_op_handler (gimple_assign_rhs_code (ass), + TREE_TYPE (gimple_assign_lhs (ass))); + if (const gcond *cond = dyn_cast (s)) + return range_op_handler (gimple_cond_code (cond), + TREE_TYPE (gimple_cond_lhs (cond))); return NULL; } diff --git a/gcc/gimple.h b/gcc/gimple.h index d359f7827b1..b935ad4f761 100644 --- a/gcc/gimple.h +++ b/gcc/gimple.h @@ -2229,26 +2229,6 @@ gimple_set_modified (gimple *s, bool modifiedp) } -/* Return the tree code for the expression computed by STMT. This is - only valid for GIMPLE_COND, GIMPLE_CALL and GIMPLE_ASSIGN. For - GIMPLE_CALL, return CALL_EXPR as the expression code for - consistency. This is useful when the caller needs to deal with the - three kinds of computation that GIMPLE supports. */ - -static inline enum tree_code -gimple_expr_code (const gimple *stmt) -{ - enum gimple_code code = gimple_code (stmt); - if (code == GIMPLE_ASSIGN || code == GIMPLE_COND) - return (enum tree_code) stmt->subcode; - else - { - gcc_gimple_checking_assert (code == GIMPLE_CALL); - return CALL_EXPR; - } -} - - /* Return true if statement STMT contains volatile operands. */ static inline bool @@ -3793,6 +3773,28 @@ gimple_cond_set_condition (gcond *stmt, enum tree_code code, tree lhs, gimple_cond_set_rhs (stmt, rhs); } + +/* Return the tree code for the expression computed by STMT. This is + only valid for GIMPLE_COND, GIMPLE_CALL and GIMPLE_ASSIGN. For + GIMPLE_CALL, return CALL_EXPR as the expression code for + consistency. This is useful when the caller needs to deal with the + three kinds of computation that GIMPLE supports. */ + +static inline enum tree_code +gimple_expr_code (const gimple *stmt) +{ + if (const gassign *ass = dyn_cast (stmt)) + return gimple_assign_rhs_code (ass); + if (const gcond *cond = dyn_cast (stmt)) + return gimple_cond_code (cond); + else + { + gcc_gimple_checking_assert (gimple_code (stmt) == GIMPLE_CALL); + return CALL_EXPR; + } +} + + /* Return the LABEL_DECL node used by GIMPLE_LABEL statement GS. */ static inline tree