From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id DA667385840A for ; Mon, 1 Nov 2021 14:21:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DA667385840A Received: from mail-lj1-f198.google.com (mail-lj1-f198.google.com [209.85.208.198]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-246-oTp_PUi-MIWcQIIak0zPkQ-1; Mon, 01 Nov 2021 10:21:16 -0400 X-MC-Unique: oTp_PUi-MIWcQIIak0zPkQ-1 Received: by mail-lj1-f198.google.com with SMTP id bn14-20020a05651c178e00b002164a557684so351105ljb.6 for ; Mon, 01 Nov 2021 07:21:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=uJCE04HNnM8KkK77DAR/jQE4w/MGIS2VpkJjqR+qVZs=; b=5n5Yar496ojPtHPdS6N+b0bW3BjHbAi2KjkNG6MJIaWQt+CYkvJpFk1vi3ZcZC8SHB sKtTdTDFiq/0fEEPH2AbL/HPGB4vCoNEFOFYLDPFheFU6OfoowBbHdIuS95v2nGtEb9r 4CmXLtA2ZWqcdiXv4Vh+BhulEPvQsY0Cnt8wnTlX1bc7cX+/V1HxPuaAZCCx99+GZx1b xIPzEUFTtCt4UsnbJfwPgImvwuJ9jAoyzg2y6lel8tFkX/va7xeL9HVs4TgWEtfovYRb d2h9Eg4IoBODVaUoQpQxENCRbAEQxRKMw1AHxtEA1acTcMDWGKxS5ERNb5/E4wrMtzu7 FWSg== X-Gm-Message-State: AOAM532AZvm1qkU5dGgsBX/Tk9UFjrw9gIKoyDxiEUa15K4EKZiDNL1d 9/i5GmYGACx9aC0BzknrZ9X8maxC+XEmOhixo/SNzGrgSe6p8V4b9OeothL4fnbHFyJFbnRxvMp mmFzDd07uuyCZfbUvNxOOaXcPmGOrY3Fveg== X-Received: by 2002:ac2:4285:: with SMTP id m5mr26876590lfh.86.1635776474583; Mon, 01 Nov 2021 07:21:14 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzXMbehSgiiAJNqfSdFYXoD7ONPHviZUc4+shG9D4efbYvHaHu3PSe8GMOpQLDMtxAuc38FiVA+8BkyYEwDdZ8= X-Received: by 2002:ac2:4285:: with SMTP id m5mr26876571lfh.86.1635776474326; Mon, 01 Nov 2021 07:21:14 -0700 (PDT) MIME-Version: 1.0 References: <20211027181323.395724-1-aldyh@redhat.com> <20211028015530.6fd3ee60@nbbrfq> <20211101151027.06945d6a@nbbrfq> In-Reply-To: <20211101151027.06945d6a@nbbrfq> From: Aldy Hernandez Date: Mon, 1 Nov 2021 15:21:03 +0100 Message-ID: Subject: Re: redundant bitmap_bit_p followed by bitmap_clear_bit [was: Re: [COMMITTED] Kill second order relations in the path solver.] To: Bernhard Reutner-Fischer Cc: Andrew MacLeod , GCC patches X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-5.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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-List-Received-Date: Mon, 01 Nov 2021 14:21:19 -0000 I'm not convinced this makes the code clearer to read, especially if it's not on a critical path. But if you feel strongly, please submit a patch ;-). Aldy On Mon, Nov 1, 2021 at 3:10 PM Bernhard Reutner-Fischer wrote: > > On Thu, 28 Oct 2021 01:55:30 +0200 > Bernhard Reutner-Fischer wrote: > > > On Wed, 27 Oct 2021 20:13:21 +0200 > > Aldy Hernandez via Gcc-patches wrote: > > > > @@ -1306,6 +1307,24 @@ path_oracle::killing_def (tree ssa) > > > ptr->m_next = m_equiv.m_next; > > > m_equiv.m_next = ptr; > > > bitmap_ior_into (m_equiv.m_names, b); > > > + > > > + // Walk the relation list an remove SSA from any relations. > > > > s/an /and / > > > > > + if (!bitmap_bit_p (m_relations.m_names, v)) > > > + return; > > > + > > > + bitmap_clear_bit (m_relations.m_names, v); > > > > IIRC bitmap_clear_bit returns true if the bit was set, false otherwise, > > so should be used as if(!bitmap_clear_bit) above. > > > > + relation_chain **prev = &(m_relations.m_head); > > > > s/[()]// > > thanks, > > There seems to be two other spots where a redundant bitmap_bit_p checks > if we want to bitmap_clear_bit. In dse and ira. > Like: > $ cat ~/coccinelle/gcc_bitmap_bit_p-0.cocci ; echo EOF > // replace redundant bitmap_bit_p() bitmap_clear_bit with the latter > @ rule1 @ > identifier fn; > expression bitmap, bit; > @@ > > fn(...) { > <... > ( > -if (bitmap_bit_p (bitmap, bit)) > +if (bitmap_clear_bit (bitmap, bit)) > { > ... > - bitmap_clear_bit (bitmap, bit); > ... > } > | > -if (bitmap_bit_p (bitmap, bit)) > +if (bitmap_clear_bit (bitmap, bit)) > { > ... > } > ... > -bitmap_clear_bit (bitmap, bit); > ) > ...> > } > EOF > $ find gcc/ -type f -a \( -name "*.c" -o -name "*.cc" \) -a \( ! -path "gcc/testsuite/*" -a ! -path "gcc/contrib/*" \) -exec spatch -sp_file ~/coccinelle/gcc_bitmap_bit_p-0.cocci --show-diff {} \; > diff = > --- gcc/dse.c > +++ /tmp/cocci-output-1104419-443759-dse.c > @@ -3238,9 +3238,8 @@ mark_reachable_blocks (sbitmap unreachab > edge e; > edge_iterator ei; > > - if (bitmap_bit_p (unreachable_blocks, bb->index)) > + if (bitmap_clear_bit(unreachable_blocks, bb->index)) > { > - bitmap_clear_bit (unreachable_blocks, bb->index); > FOR_EACH_EDGE (e, ei, bb->preds) > { > mark_reachable_blocks (unreachable_blocks, e->src); > diff = > --- gcc/ira.c > +++ /tmp/cocci-output-1104678-d8679a-ira.c > @@ -2944,17 +2944,15 @@ mark_elimination (int from, int to) > FOR_EACH_BB_FN (bb, cfun) > { > r = DF_LR_IN (bb); > - if (bitmap_bit_p (r, from)) > + if (bitmap_clear_bit(r, from)) > { > - bitmap_clear_bit (r, from); > bitmap_set_bit (r, to); > } > if (! df_live) > continue; > r = DF_LIVE_IN (bb); > - if (bitmap_bit_p (r, from)) > + if (bitmap_clear_bit(r, from)) > { > - bitmap_clear_bit (r, from); > bitmap_set_bit (r, to); > } > } > # in ira.c one would have to fixup the curly braces manually > PS: coccinelle seems to ruin the spaces before braces in the '+' even > though i have written them correctly according to GNU style.. >