| Message ID | 20180914040649.1794-3-joel@jms.id.au (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | powerpc: Clang build fixes | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | next/apply_patch Successfully applied |
| snowpatch_ozlabs/checkpatch | success | Test checkpatch on branch next |
On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote: > > Clang's assembler does not like the syntax of the cmpdi: > > arch/powerpc/boot/crt0.S:168:22: error: unexpected modifier on variable reference > cmpdi 12,RELACOUNT@l > ^ > arch/powerpc/boot/crt0.S:168:11: error: unknown operand > cmpdi 12,RELACOUNT@l > ^ > Enclosing RELACOUNT in () makes it is happy. Tested with GCC 8 and Clang > 8 (trunk). > > Reported to clang as https://bugs.llvm.org/show_bug.cgi?id=38945 > > Signed-off-by: Joel Stanley <joel@jms.id.au> > --- > v2: Fix for !powerpc64 too, add bug link to commit message > --- > arch/powerpc/boot/crt0.S | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/boot/crt0.S b/arch/powerpc/boot/crt0.S > index dcf2f15e6797..e453e011d7e7 100644 > --- a/arch/powerpc/boot/crt0.S > +++ b/arch/powerpc/boot/crt0.S > @@ -80,7 +80,7 @@ p_base: mflr r10 /* r10 now points to runtime addr of p_base */ > lwz r9,4(r12) /* get RELA pointer in r9 */ > b 12f > 11: addis r8,r8,(-RELACOUNT)@ha > - cmpwi r8,RELACOUNT@l > + cmpwi r8,(RELACOUNT)@l > bne 12f > lwz r0,4(r12) /* get RELACOUNT value in r0 */ > 12: addi r12,r12,8 > @@ -165,7 +165,7 @@ p_base: mflr r10 /* r10 now points to runtime addr of p_base */ > ld r13,8(r11) /* get RELA pointer in r13 */ > b 11f > 10: addis r12,r12,(-RELACOUNT)@ha > - cmpdi r12,RELACOUNT@l > + cmpdi r12,(RELACOUNT)@l Yep, as we can see above, when RELACOUNT is negated, it's wrapped in parens. It's important for Clang's assembler to match GAS eventually, but for now, this change simply cononicalizes all of the the references to RELACOUNT in this source file. Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > bne 11f > ld r8,8(r11) /* get RELACOUNT value in r8 */ > 11: addi r11,r11,16 > -- > 2.17.1 >
On Fri, Sep 14, 2018 at 10:47:08AM -0700, Nick Desaulniers wrote: > On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote: > > 10: addis r12,r12,(-RELACOUNT)@ha > > - cmpdi r12,RELACOUNT@l > > + cmpdi r12,(RELACOUNT)@l > > Yep, as we can see above, when RELACOUNT is negated, it's wrapped in > parens. The only thing that does is make it easier for humans to read; it means exactly the same thing. Segher
On Fri, Sep 14, 2018 at 2:08 PM Segher Boessenkool <segher@kernel.crashing.org> wrote: > > On Fri, Sep 14, 2018 at 10:47:08AM -0700, Nick Desaulniers wrote: > > On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote: > > > 10: addis r12,r12,(-RELACOUNT)@ha > > > - cmpdi r12,RELACOUNT@l > > > + cmpdi r12,(RELACOUNT)@l > > > > Yep, as we can see above, when RELACOUNT is negated, it's wrapped in > > parens. Looks like this was just fixed in Clang-8: https://bugs.llvm.org/show_bug.cgi?id=38945 https://reviews.llvm.org/D52188
On Tue, 18 Sep 2018 at 06:11, Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Fri, Sep 14, 2018 at 2:08 PM Segher Boessenkool > <segher@kernel.crashing.org> wrote: > > > > On Fri, Sep 14, 2018 at 10:47:08AM -0700, Nick Desaulniers wrote: > > > On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote: > > > > 10: addis r12,r12,(-RELACOUNT)@ha > > > > - cmpdi r12,RELACOUNT@l > > > > + cmpdi r12,(RELACOUNT)@l > > > > > > Yep, as we can see above, when RELACOUNT is negated, it's wrapped in > > > parens. > > Looks like this was just fixed in Clang-8: > https://bugs.llvm.org/show_bug.cgi?id=38945 > https://reviews.llvm.org/D52188 Nice! mpe, given we need the local references to labels fix which is also in clang-8 I suggest we drop this patch. Cheers, Joel
Joel Stanley <joel@jms.id.au> writes: > On Tue, 18 Sep 2018 at 06:11, Nick Desaulniers <ndesaulniers@google.com> wrote: >> >> On Fri, Sep 14, 2018 at 2:08 PM Segher Boessenkool >> <segher@kernel.crashing.org> wrote: >> > >> > On Fri, Sep 14, 2018 at 10:47:08AM -0700, Nick Desaulniers wrote: >> > > On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote: >> > > > 10: addis r12,r12,(-RELACOUNT)@ha >> > > > - cmpdi r12,RELACOUNT@l >> > > > + cmpdi r12,(RELACOUNT)@l >> > > >> > > Yep, as we can see above, when RELACOUNT is negated, it's wrapped in >> > > parens. >> >> Looks like this was just fixed in Clang-8: >> https://bugs.llvm.org/show_bug.cgi?id=38945 >> https://reviews.llvm.org/D52188 > > Nice! > > mpe, given we need the local references to labels fix which is also in > clang-8 I suggest we drop this patch. OK, no worries. cheers
diff --git a/arch/powerpc/boot/crt0.S b/arch/powerpc/boot/crt0.S index dcf2f15e6797..e453e011d7e7 100644 --- a/arch/powerpc/boot/crt0.S +++ b/arch/powerpc/boot/crt0.S @@ -80,7 +80,7 @@ p_base: mflr r10 /* r10 now points to runtime addr of p_base */ lwz r9,4(r12) /* get RELA pointer in r9 */ b 12f 11: addis r8,r8,(-RELACOUNT)@ha - cmpwi r8,RELACOUNT@l + cmpwi r8,(RELACOUNT)@l bne 12f lwz r0,4(r12) /* get RELACOUNT value in r0 */ 12: addi r12,r12,8 @@ -165,7 +165,7 @@ p_base: mflr r10 /* r10 now points to runtime addr of p_base */ ld r13,8(r11) /* get RELA pointer in r13 */ b 11f 10: addis r12,r12,(-RELACOUNT)@ha - cmpdi r12,RELACOUNT@l + cmpdi r12,(RELACOUNT)@l bne 11f ld r8,8(r11) /* get RELACOUNT value in r8 */ 11: addi r11,r11,16
Clang's assembler does not like the syntax of the cmpdi: arch/powerpc/boot/crt0.S:168:22: error: unexpected modifier on variable reference cmpdi 12,RELACOUNT@l ^ arch/powerpc/boot/crt0.S:168:11: error: unknown operand cmpdi 12,RELACOUNT@l ^ Enclosing RELACOUNT in () makes it is happy. Tested with GCC 8 and Clang 8 (trunk). Reported to clang as https://bugs.llvm.org/show_bug.cgi?id=38945 Signed-off-by: Joel Stanley <joel@jms.id.au> --- v2: Fix for !powerpc64 too, add bug link to commit message --- arch/powerpc/boot/crt0.S | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)