From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 83549 invoked by alias); 14 Aug 2015 17:08:39 -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 83526 invoked by uid 89); 14 Aug 2015 17:08:38 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: fencepost.gnu.org Received: from fencepost.gnu.org (HELO fencepost.gnu.org) (208.118.235.10) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Fri, 14 Aug 2015 17:08:36 +0000 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40195) by fencepost.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1ZQIT4-0006lE-A9 for gcc-patches@gnu.org; Fri, 14 Aug 2015 13:08:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZQIT0-0006On-Px for gcc-patches@gnu.org; Fri, 14 Aug 2015 13:08:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46013) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZQIT0-0006OY-KA for gcc-patches@gnu.org; Fri, 14 Aug 2015 13:08:30 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id D4F048E248; Fri, 14 Aug 2015 17:08:27 +0000 (UTC) Received: from localhost.localdomain (ovpn-113-104.phx2.redhat.com [10.3.113.104]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t7EH8RRC009282; Fri, 14 Aug 2015 13:08:27 -0400 Subject: Re: [PATCH 1/2] C++-ify dominance.c To: Richard Biener , Mikhail Maltsev References: <55CD3C87.40101@gmail.com> Cc: gcc-patches From: Jeff Law Message-ID: <55CE208B.3010304@redhat.com> Date: Fri, 14 Aug 2015 17:26:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 X-IsSubscribed: yes X-SW-Source: 2015-08/txt/msg00817.txt.bz2 On 08/14/2015 01:54 AM, Richard Biener wrote: > On Fri, Aug 14, 2015 at 2:55 AM, Mikhail Maltsev wrote: >> Hi all. >> >> These two patches are refactoring of dominator-related code. >> >> The comment in dominance.c says: "We work in a poor-mans object oriented >> fashion, and carry an instance of this structure through all our 'methods'". So, >> the first patch converts the mentioned structure (dom_info) into a class with >> proper encapsulation. It also adds a new member - m_fn (the function currently >> being compiled) to this structure and replaces some uses of cfun with m_fn. It >> also contains some fixes, related to current coding standards: move variable >> declarations to place of first use, replace elaborated type specifiers (i.e. >> "struct/enum foo") by simple ones (i.e., just "foo") in function prototypes. > > Putting in m_fn looks backwards to me - it looks like we only need to remember > the entry and exit BBs and the number of blocks. In fact initializing > dom_info from that would allow it to work on SESE regions as well? Or a SEME region. While I don't have an immediate use for dominators regions in the CFG, I believe they'd be useful. > > +unsigned bb_dom_dfs_in (cdi_direction, basic_block); > +unsigned bb_dom_dfs_out (cdi_direction, basic_block); > +extern void verify_dominators (cdi_direction); > +basic_block recompute_dominator (cdi_direction, basic_block); > +extern void iterate_fix_dominators (cdi_direction, vec , bool); > +extern void add_to_dominance_info (cdi_direction, basic_block); > > if you are here please fix the 'extern' vs. w/o 'extern' inconsistencies as well > (we prefer 'extern'). Presumably we're not at a point where we can push these down as methods in the class? ie, are we providing a class to query and manipulate the dominator tree from outside dominance.c or are we doing it just for internal stuff. I believe the former is more strategic, the latter may have value as well, but I believe it's more limited. > > In general I'm biased and refactoring for the sake of refactoring > doesn't go well with me ... But if we're taking something that's essentially C++ implemented in C and turn it into real C++ with encapsulation, that's a step in the right direction to me. jeff