Bill Wendling <mo...@google.com> writes: > On Mon, Nov 23, 2020 at 7:44 PM Michael Ellerman <m...@ellerman.id.au> wrote: >> Bill Wendling <mo...@google.com> writes: >> > On Mon, Nov 23, 2020 at 12:10 PM Segher Boessenkool >> > <seg...@kernel.crashing.org> wrote: >> >> On Mon, Nov 23, 2020 at 12:01:01PM -0800, Bill Wendling wrote: >> >> > On Mon, Nov 23, 2020 at 11:58 AM Segher Boessenkool >> >> > <seg...@kernel.crashing.org> wrote: >> >> > > > On Sun, Nov 22, 2020 at 10:36 PM Segher Boessenkool >> >> > > > <seg...@kernel.crashing.org> wrote: >> >> > > > > "true" (as a result of a comparison) in as is -1, not 1. >> >> > > >> >> > > On Mon, Nov 23, 2020 at 11:43:11AM -0800, Bill Wendling wrote: >> >> > > > What Segher said. :-) Also, if you reverse the comparison, you'll >> >> > > > get >> >> > > > a build error. >> >> > > >> >> > > But that means your patch is the wrong way around? >> >> > > >> >> > > - .ifgt (label##4b- label##3b)-(label##2b- label##1b); \ >> >> > > - .error "Feature section else case larger than body"; \ >> >> > > - .endif; \ >> >> > > + .org . - ((label##4b-label##3b) > (label##2b-label##1b)); \ >> >> > > >> >> > > It should be a + in that last line, not a -. >> >> > >> >> > I said so in a follow up email. >> >> >> >> Yeah, and that arrived a second after I pressed "send" :-) >> >> >> > Michael, I apologize for the churn with these patches. I believe the >> > policy is to resend the match as "v4", correct? >> > >> > I ran tests with the change above. It compiled with no error. If I >> > switch the labels around to ".org . + ((label##2b-label##1b) > >> > (label##4b-label##3b))", then it fails as expected. >> >> I wanted to retain the nicer error reporting for gcc builds, so I did it >> like this: >> >> diff --git a/arch/powerpc/include/asm/feature-fixups.h >> b/arch/powerpc/include/asm/feature-fixups.h >> index b0af97add751..c4ad33074df5 100644 >> --- a/arch/powerpc/include/asm/feature-fixups.h >> +++ b/arch/powerpc/include/asm/feature-fixups.h >> @@ -36,6 +36,24 @@ label##2: \ >> .align 2; \ >> label##3: >> >> + >> +#ifndef CONFIG_CC_IS_CLANG >> +#define CHECK_ALT_SIZE(else_size, body_size) \ >> + .ifgt (else_size) - (body_size); \ >> + .error "Feature section else case larger than body"; \ >> + .endif; >> +#else >> +/* >> + * If we use the ifgt syntax above, clang's assembler complains about the >> + * expression being non-absolute when the code appears in an inline assembly >> + * statement. >> + * As a workaround use an .org directive that has no effect if the else case >> + * instructions are smaller than the body, but fails otherwise. >> + */ >> +#define CHECK_ALT_SIZE(else_size, body_size) \ >> + .org . + ((else_size) > (body_size)); >> +#endif >> + >> #define MAKE_FTR_SECTION_ENTRY(msk, val, label, sect) \ >> label##4: \ >> .popsection; \ >> @@ -48,9 +66,7 @@ label##5: >> \ >> FTR_ENTRY_OFFSET label##2b-label##5b; \ >> FTR_ENTRY_OFFSET label##3b-label##5b; \ >> FTR_ENTRY_OFFSET label##4b-label##5b; \ >> - .ifgt (label##4b- label##3b)-(label##2b- label##1b); \ >> - .error "Feature section else case larger than body"; \ >> - .endif; \ >> + CHECK_ALT_SIZE((label##4b-label##3b), (label##2b-label##1b)); \ >> .popsection; >> >> >> >> I've pushed a branch with all your patches applied to: >> >> https://github.com/linuxppc/linux/commits/next-test >> > This works for me. Thanks!
Great. >> Are you able to give that a quick test? It builds clean with clang for >> me, but we must be using different versions of clang because my branch >> already builds clean for me even without your patches. >> > You may need to set LLVM_IAS=1 to get the behavior I'm seeing. That > turns on clang's integrated assembler, which I think is disabled by > default. Yep that does it. But then I get: clang: error: unsupported argument '-mpower4' to option 'Wa,' clang: error: unsupported argument '-many' to option 'Wa,' So I guess I'm still missing something? > Note that with clang's integrated assembler, arch/powerpc/boot/util.S > fails to compile. Alan Modra mentioned that he sent you a patch to > "modernize" the file so that clang can compile it. Ah you're right he did, it didn't go to patchwork so I missed it. Have grabbed it now. cheers
