From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 106980 invoked by alias); 20 Oct 2016 13:55:51 -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 106957 invoked by uid 89); 20 Oct 2016 13:55:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=1046, kits, obj1, 1047 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 20 Oct 2016 13:55:49 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 61865C05678F for ; Thu, 20 Oct 2016 13:55:48 +0000 (UTC) Received: from vpn-238-138.phx2.redhat.com (vpn-238-138.phx2.redhat.com [10.3.238.138]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9KDtlpB016812; Thu, 20 Oct 2016 09:55:47 -0400 Message-ID: <1476971747.28626.7.camel@redhat.com> Subject: INSN_UIDs again (was Re: [PATCH 15/16] RTL frontend (rtl1), on top of dump reader) From: David Malcolm To: Bernd Schmidt , gcc-patches@gcc.gnu.org Date: Thu, 20 Oct 2016 13:55:00 -0000 In-Reply-To: References: <1475684110-2521-1-git-send-email-dmalcolm@redhat.com> <1475684110-2521-16-git-send-email-dmalcolm@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-10/txt/msg01643.txt.bz2 On Thu, 2016-10-06 at 15:30 +0200, Bernd Schmidt wrote: > On 10/05/2016 06:15 PM, David Malcolm wrote: > > +;; MEM[(struct isl_obj *)&obj1] = &isl_obj_map_vtable; > > +(insn 1045 0 1046 2 (set (reg:SI 480) > > + (high:SI (symbol_ref:SI ("isl_obj_map_vtable") > > + [flags 0xc0] > > + > isl_obj_map_vtable>))) > > + y.c:12702 -1 > > + (nil)) > > +(insn 1046 1045 1047 2 (set (reg/f:SI 479) > > + (lo_sum:SI (reg:SI 480) > > + (symbol_ref:SI ("isl_obj_map_vtable") > > + [flags 0xc0] > > + ))) > > + y.c:12702 -1 > > + (expr_list:REG_EQUAL (symbol_ref:SI ("isl_obj_map_vtable") > > + [flags 0xc0] > > + > isl_obj_map_vtable>) > > + (nil))) > > I hate saying this, since you've obviously spent a lot of effort, but > I > think we're still at a too early stage to construct testcases. One > issue > that came up while discussing previous patch kits is that the format > here is still too verbose, and I'd like to settle on a format first. > There's all manner of things that are pointless in a testcase and > impede > readability: > > * INSN_UIDs and NEXT/PREV fields (could be constructed from > scratch, except for labels) Currently (as of r241120) the "compact" dump format omits NEXT/PRIV fields, and only dumps INSN_UIDs for CODE_LABELs (and within LABEL_REFs). I've reimplemented the loader so that it copes with this, preserving INSN_UIDs where provided, and generating others, ensuring uniqueness. I found myself making this change to a test case (gcc.dg/rtl/x86_64/into-cfglayout.c): /* The conversion to cfglayout should eliminate unconditional jump instructions... */ -/* { dg-final { scan-rtl-dump "Removing jump 14." "into_cfglayout" } } */ -/* { dg-final { scan-rtl-dump-not "jump_insn 14" "into_cfglayout" } } */ -/* { dg-final { scan-rtl-dump-not "barrier 15" "into_cfglayout" } } */ +/* { dg-final { scan-rtl-dump "Removing jump 13." "into_cfglayout" } } */ +/* { dg-final { scan-rtl-dump-not "jump_insn 13" "into_cfglayout" } } */ +/* { dg-final { scan-rtl-dump-not "barrier" "into_cfglayout" } } */ /* ...but conditional jumps should be preserved. */ -/* { dg-final { scan-rtl-dump "jump_insn 10" "into_cfglayout" } } */ +/* { dg-final { scan-rtl-dump "jump_insn 9" "into_cfglayout" } } */ All of the INSN_UIDs being referred to are implicitly generated by the loader. Presumably we want to allow for test cases like this that verify that a specific insn was affected by a pass e.g. scanning for "Removing jump 14", but having the INSN_UIDs be autogenerated seems to make this very fragile (and makes the scan-rtl-dump directives hard to read). Hence I'm leaning towards having the dump format contain explicit INSN_UIDs for all insns, not just CODE_LABELs (but omitting the NEXT/PREV, those are redundant noise). Currently the jump insn in question looks like this: (cjump_insn (set (pc) (label_ref 20)) (nil)) With explicit INSN_UIDs it would look like this: (cjump_insn 13 (set (pc) (label_ref 20)) (nil)) Thoughts? Dave > * INSN_CODE_NUMBERs in both textual and number form. > * Various (nil) fields > * VAR_DECL addresses. [...snip...]