Hi Segher, Thanks for the review!
on 2022/8/19 01:34, Segher Boessenkool wrote: > Hi! > > On Thu, Aug 18, 2022 at 10:12:48AM +0800, Kewen.Lin wrote: >> As PR99888 and its related show, the current support for >> -fpatchable-function-entry on powerpc ELFv2 doesn't work >> well with global entry existence. > >> + /* Emit the NOPs after local entry. */ > > Please do not say "NOPs". It is not an acronym. I know some of our > documentation has this bug already, but please do not spread it further. > > The machine instruction is "nop", lowercase. > > Please fix this. Whoops, I thought it's only used in commit log mistakenly, will fix! > > So, this patch overloads the meaning of the two parameters here to have > more meaning than explained in the documentation for the option. There > isn't much that can be done about this, so adding some new option would > only be extra work for everyone. But, could you add a line or two in > the documentation? "For PowerPC with the ELFv2 ABI, there will be M > nops before the local entry point, and N-M after", something like that? > Since you proposed to update the documentation, I'm thinking if we can reconsider Fangrui's proposal in the PR which Alan seconded: Put preceding nops before GEP and succeeding nops after LEP. Previously I had the concern that the nops inserted doesn't respect to a same function entry, it looks inconsistent to the documentation, and you also noted that "The nops have to be consecutive". If we want to update the documentation, could we reword it for PowerPC ELFv2 ABI? What's your opinion? BR, Kewen
