On Sun, Oct 11, 2020 at 01:12:40PM -0400, Tom Lane wrote: > Noah Misch <n...@leadboat.com> writes: > > The first attachment fixes the matter you've reported. While confirming > > that, > > I observed that gcc builds don't even use the 64-bit code in arch-ppc.h. > > Oops. The second attachment fixes that. > > I reviewed these, and tested the first one on a nearby Apple machine. > (I lack access to 64-bit PPC, so I can't actually test the second.) > They look fine, and I confirmed by examining asm output that even > the rather-old-now gcc version that Apple last shipped for PPC does > the right thing with the conditionals.
Thanks for reviewing and for mentioning that old-gcc behavior. I had a comment asserting that gcc 7.2.0 didn't deduce constancy from those conditionals. Checking again now, it was just $SUBJECT preventing constancy deduction. I made the patch remove that comment. > > I plan not to back-patch either of these. > > Hmm, I'd argue for a back-patch. The issue of modern compilers > warning about the incorrect code will apply to all supported branches. > Moreover, even if we don't use these code paths today, who's to say > that someone won't back-patch a bug fix that requires them? I do not > think it's unreasonable to expect these functions to work well in > all branches that have them. Okay, I've pushed with a back-patch. compare_exchange-ppc-immediate-v1.patch affects on code generation are limited to regress.o, so it's quite safe to back-patch. I just didn't think it was standard to back-patch for the purpose of removing a -Wsign-compare warning. (Every branch is noisy under -Wsign-compare.) atomics-ppc64-gcc-v1.patch does change code generation, in the manner discussed in the big arch-ppc.h comment (starts with "This mimics gcc"). Still, I've accepted the modest risk.
