On Sep 13, 2025, Segher Boessenkool <seg...@kernel.crashing.org> wrote:

> Hi!
> On Sat, Sep 13, 2025 at 12:07:33AM -0300, Alexandre Oliva wrote:

>> Tested manually with cross binutils for ppc64-linux-gnu and
>> ppc64-vxworks7r2.

> "Tested manually", what does that mean?

It means I fed the fragments (before and after) to the assembler and
linker, as run in that code path in configure, and got the same results
(the warning used to be discarded).

Given the same results, there's no functional change to the compiler.
I'm only removing a source of confusion.

I've also ran various builds with that patch in, in case that matters
for such a trivial change.  Even ppc targets.

>> Unless there's any objection in the next few days, I
>> plan to put this in along with other changes.

> You cannot do that, ever.

Of course I can.  I'm a co-maintainer of the modified files, and I have
been for almost 3 decades.  Even if I weren't, the error and the fix are
pretty obvious for anyone acquainted with ppc codegen, register
assignments and relocations.

> The patch itself looks fine, but the commit message doesn't

I don't see anything wrong with the single-paragraph commit message, and
you seemed to agree with it.  What are you getting at?  Are you by any
chance under the mistaken notion that the description of how I tested
the patch and when I intended to commit it was part of a commit message?

> and the changelog not either:

>> for  gcc/ChangeLog
>> 
>> * configure.ac: Adjust base register in linker test for large
>> toc support.
>> * configure: Rebuilt.

> You should never use passive tense in changelogs.

I'm familiar with that theory.  Practice has had the tense of this
specific verb split about evenly in the top-level Changelog.  But since
you seem to feel strongly about it, I'll adjust the spelling.

> Oh, and changelog
> lines are 80 character positions long, please don't wrap early.

The top-level ChangeLog says 76; gcc/ChangeLog doesn't override the
default.  Why does that matter, it's not even like we'd save a line or
anything...  Also, there are plenty of commit messages that don't go
past the earlier 70-columns standard.  When and how did it change,
anyway?

I'd even argue that it's good not to use too-long lines in ChangeLog
entries that go in the commit log, because git indents that, so they'd
go past the 80-columns standard terminal width.  I feel pretty strongly
about that myself, so I'm not changing that.

> And "TOC" is an initialism, spelled in all capitals.

I don't usually care much about capitalization, and I'm sure I can find
plenty of examples of ppc's ToC spelled with various different
capitalizations, but if you feel strongly about it, I don't mind
tweaking it to your liking (done), provided that nobody else objects to
the alternative.


Here's what the relevant part of the git log output now looks like:

    [ppc] adjust configure test for large TOC support
    
    The use of the TLS register in a TOC/GOT address computation was
    probably a cut&pasto or a thinko.  It causes a linker warning and,
    because the TLS access in the test is incomplete, may cause
    significant confusion.  Adjust to use the TOC/GOT register as base.
    
    
    for  gcc/ChangeLog
    
            * configure.ac: Adjust base register in linker test for large
            TOC support.
            * configure: Rebuild.


-- 
Alexandre Oliva, happy hacker            https://blog.lx.oliva.nom.br/
Free Software Activist     FSFLA co-founder     GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity.
Excluding neuro-others for not behaving ""normal"" is *not* inclusive!

Reply via email to