On Wed, Mar 25, 2020 at 11:07:04AM -0500, will schmidt wrote: > Subject: [Patch v327] set -mcprel by default ... > > Include some powerpc or rs6000 reference in the subject. My filters > missed this one. (I'm assuming this is powerpc target specific).
rs6000: Enable pcrel by default (etc.) Subjects start with a capital, but end without dot. Just like in normal email. > > This is a revision of a patch that I've submitted several times. It makes > > -mpcrel the default on Linux 64-bit systems that use ELF v2, use the medium > > code mode, and if the user did not disable prefixed load/store instructions > > for > > -mcpu=future. > > The fewer words, the easier to review. History is important but so is > making the review easy. > > "This patch makes -mpcrel the default on Linux 64-bit systems that use > ELF V2, medium code model, and have not disabled prefix instructions." Yes :-) But, don't say "This patch", if you can avoid it... just use an imperative: "Make -mpcrel the default ..." > Is there need or desire to explicitly set TARGET_FUTURE or > TARGET_PREFIXED to zero in the header file now? There are no other > references to those in the header file. It is automatically defined as #define TARGET_FUTURE ((rs6000_isa_flags & OPTION_MASK_FUTURE) != 0) so that will work just fine. Thanks for the review Will. Mike, could you send a patch with those fixes please? Make sure the changelog agrees with the patch (and don't say "why" in changelog -- say that in the commit message. Which is free form, so you have much more freedom to explain things in a useful order). Segher
