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