From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-x532.google.com (mail-pg1-x532.google.com [IPv6:2607:f8b0:4864:20::532]) by sourceware.org (Postfix) with ESMTPS id AC2863858C53 for ; Tue, 26 Apr 2022 18:05:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AC2863858C53 Received: by mail-pg1-x532.google.com with SMTP id v10so5609341pgl.11 for ; Tue, 26 Apr 2022 11:05:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=7wcGCoJ6I05oWhDgIwbUEqEw1LG9jPC32L5B8VosZfI=; b=xpDd5o8CrMM5/HYZrHiSigXRl42DBvod4x0vlrgURL8MYpY8aJ11s1J4icwN3y3Hlj CMtlyq8bU+fbT9BNNdyhKCi/7J3H2bqFIbsXgKjp4z+xuUG1BgxaqQUwRgreNAhltBPX aM1JhpQ8plclHKlcH6Rfg2z1D/zyZsWf+4XdcHXA1eG/nb7UvmywOb5xqw1NeXh+EdB6 RQ37wWm7/Nm3hfBJvRh+vg3Wob8rFho5A+tidz0fXjpF9h9ukpkKKYC+7jXe9b6U4FmL 4A2FJGfWE0xBln32I56nXORg/R1DcKrOsDbcvohE3yw5yOEXVoz5jVlKWlEnLEzkO+LH OQUQ== X-Gm-Message-State: AOAM533RdnLLG+RBdXBecD2RussT7WHGqtyXLkpN1nymcQ36HEX0rti+ lRpFe6pbyzh2EyEMUsa94f8J X-Google-Smtp-Source: ABdhPJxjtv4r8v29JQaWSrUj3FYxE3umjXIUv64g0NFBz4/gLO5EKXKOubcWx7Mj+rmRpTMSSPfWCg== X-Received: by 2002:aa7:9283:0:b0:505:fe1e:f6b with SMTP id j3-20020aa79283000000b00505fe1e0f6bmr25858478pfa.29.1650996347708; Tue, 26 Apr 2022 11:05:47 -0700 (PDT) Received: from takamaka.home ([184.69.131.86]) by smtp.gmail.com with ESMTPSA id v189-20020a622fc6000000b004fb72e95806sm15780482pfv.48.2022.04.26.11.05.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 26 Apr 2022 11:05:46 -0700 (PDT) Received: by takamaka.home (Postfix, from userid 1000) id 1A7CFA1626; Tue, 26 Apr 2022 11:05:46 -0700 (PDT) Date: Tue, 26 Apr 2022 11:05:46 -0700 From: Joel Brobecker To: Carl Love Cc: Joel Brobecker , will schmidt , gdb-patches@sourceware.org, Ulrich Weigand , Tulio Magno , Rogerio Alves Subject: Re: [PATCH 1/2 Version 6] Add recording support for the ISA 3.1 Powerpc instructions Message-ID: References: <7ff0655bead2a8245018fbf9624f207cd38a5b7f.camel@us.ibm.com> <5759ca3fe3e927d36d9637a0e6456450cd18b0d5.camel@vnet.ibm.com> <762a83f5b7aed54747622c3aeaf2580d5e2db3b7.camel@us.ibm.com> <219ce4cb0701aae527ea47db2efede2a8d1cafd9.camel@us.ibm.com> <806f06b987bdab9da6babc14f296d6944b220d68.camel@us.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <806f06b987bdab9da6babc14f296d6944b220d68.camel@us.ibm.com> X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, 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: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Apr 2022 18:05:50 -0000 > Thanks for the additional comments. Given that we are not under any > timeline to get this patch done and the changes are very minor I think > it is better to just get it right the first time rather than have to > come back ad fix it up later. The changes are all localized as well. That's a great attitude to have. For large patches, it can have a bit of a perverse effect, though, which is related to how code review is done for GDB (email-based). So, to review a new iteration of a patch I already reviewed before, when the patch is large, I tend to go fish in the archives the previous iteration, and do a diff between previous and new. Easy enough, but it requires a bit of time to do so. If I trust a person enough to follow up, or when things are very minor, I tend to let the patch through, and ask that the corrections be done as a followup patch. It's true that this means two patches instead of one, so this could be seen as a real downside -- but the upside is that the minor updates are sent for review on their own, and very often, reviewing just that update patch will be trivial, and therefore the chances of it being reviewed are much higher in my experience. I was able to take a bit of time today to review the new version, though, and this version looks good to me. So you can go ahead and push to master. Thanks again. > I have made the changes to the header comment for the function > ppc_record_ACC_fpscr() to clairfy what the function does as well as to > use the capitalization to indicate the value. The parameter "at" was > also changed to "entry" to make it clearer we are selecting one of the > possible AT entries to be recorded. Finally, argument save_fpscr was > changed to a boolean thus eliminating the #defines for RECORD_FPSCR and > DO_NOT_RECORD_FPSCR. Fortunately, the changes are all at the beginning > of the patch making it easy to find. > > As always, the patch was retested to make sure I didn't break anything. > Please let me know if the additional changes are acceptable. Thanks. -- Joel