From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17963 invoked by alias); 16 Sep 2016 17:54:15 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 17947 invoked by uid 89); 16 Sep 2016 17:54:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=WHICH, synopsys, (unknown), Synopsys 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; Fri, 16 Sep 2016 17:54:13 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (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 439E664080; Fri, 16 Sep 2016 17:54:12 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u8GHsAbR027677; Fri, 16 Sep 2016 13:54:10 -0400 From: Pedro Alves Subject: Re: [PATCH v2] arc: New Synopsys ARC port To: Anton Kolesov , gdb-patches@sourceware.org References: <2cdb1ba5-ab28-d913-0c55-aa67c88b6e26@redhat.com> <1474045878-3850-1-git-send-email-Anton.Kolesov@synopsys.com> Cc: Francois Bedard Message-ID: Date: Fri, 16 Sep 2016 17:54:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <1474045878-3850-1-git-send-email-Anton.Kolesov@synopsys.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-09/txt/msg00187.txt.bz2 On 09/16/2016 06:11 PM, Anton Kolesov wrote: > Changes in V2: > > * Rename arc_unwind_cache struct to arc_prologue_cache. > * Rename arc_frame_cache to arc_make_frame_cache. > * Remove arc_create_cache - inline its code into the only place where it is > invoked. > * Do not manually init arc_prologue_cache fields to zero. > * Use character arrays instead of char pointers for XML feature names. > * Fix indentation of structure definitions. > * Fix off-by-one errors when iterating over register numbers. > * Fix various errors in comments. > * Add "Implement the XXX gdbarch method" comments where appropriate. > * Reformat ?: expressions. > * Use gdb_byte instead of char where appropriate. > * Use gdb_insn_length instead of a custom function that does the same. > Remove related functions which are not useful anymore: > arc_scream_to_the_void, arc_read_memory_for_disassembler, > arc_initialize_disassembler. > * Replace invalid implicit casts to bool (for example replace "!sal.line" > with "sal.line == 0"). > * Don't make returned BLINK value a constant in arc_frame_prev_register. > * Remove unnecessary "name" local variable from arc_tdesc_init. > * Remove empty gdbarch_tdep structure for ARC. > * Remove inappropriate _initialize_arc_tdep declaration from arc-tdep.h. > Add it to arc-tdep.c to avoid compiler warnings. > * Remove excessive requirements on register numbers in XML target > descriptions. > * Remove fictional LIMM and RESERVED regnums. > * Remove LP_START and LP_END from aux-minimal feature. Those registers are > not required, and they were there to fill space between PC and STATUS32, > however that is not required anymore. > * Rename BYTES_IN_REGISTER to ARC_REGISTER_SIZE. > * Add a comment to arc_delayed_print_insn to document a case that null > exec_bfd is handled safely in disassembler. > * Replace two cases of size_t usage as a loop iterator over regnums. > Regnums are universally an "int" in GDB. > * Use std::max and std::min. Thanks much. Looks very good now. I'm impressed! Once Eli has OKed the documentation, this is good to go. Noticed what looks like a small typo: > diff --git a/gdb/features/Makefile b/gdb/features/Makefile > index 809c811..6b8557b 100644 > --- a/gdb/features/Makefile > +++ b/gdb/features/Makefile > @@ -148,6 +148,8 @@ OUTPUTS = $(patsubst %,$(outdir)/%.dat,$(WHICH)) > # to make on the command line. > XMLTOC = \ > aarch64.xml \ > + arc/arc.v2.xml \ The file seems to be named "arc-v2.xml" instead. > + > + > + > + > + > + > + > + > + > + > + > + I see you're only adding descriptions with the slacks in place. That will be necessary as fallback descriptions when debugging against older stubs that don't report target descriptions, though new stubs that do support xml description don't really need to leave those spaces. Since other projects tend to copy gdb's target descriptions (the license is permissive exactly to allow that), it'd be nice to at least add a comment to the top of the files saying something to the effect. Thanks, Pedro Alves