[PATCH 0/2] Fixes for clang/lld

[PATCH 0/2] Fixes for clang/lld

[Bill Wendling]

These patches fix some compilation / linking issues with clang & lld.

Bill Wendling (2):
  powerpc/wrapper: Add "-z notext" flag to disable diagnostic
  powerpc/boot: Use clang when CC is clang

 arch/powerpc/boot/Makefile | 4 ++++
 arch/powerpc/boot/wrapper  | 6 ++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

-- 
2.29.0.rc1.297.gfa9743e501-goog

[PATCH 1/2] powerpc/wrapper: Add "-z notext" flag to disable diagnostic

[Bill Wendling]

The "-z notext" flag disables reporting an error if DT_TEXTREL is set.

  ld.lld: error: can't create dynamic relocation R_PPC64_ADDR64 against
    symbol: _start in readonly segment; recompile object files with
    -fPIC or pass '-Wl,-z,notext' to allow text relocations in the
    output
  >>> defined in
  >>> referenced by crt0.o:(.text+0x8) in archive arch/powerpc/boot/wrapper.a

The BFD linker disables this by default (though it's configurable in
current versions). LLD enables this by default. So we add the flag to
keep LLD from emitting the error.

Cc: Fangrui Song <maskray@google.com>
Cc: Alan Modra <amodra@gmail.com>
Signed-off-by: Bill Wendling <morbo@google.com>
---
 arch/powerpc/boot/wrapper | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/boot/wrapper b/arch/powerpc/boot/wrapper
index cd58a62e810d..e576d3e4b23f 100755
--- a/arch/powerpc/boot/wrapper
+++ b/arch/powerpc/boot/wrapper
@@ -46,6 +46,7 @@ compression=.gz
 uboot_comp=gzip
 pie=
 format=
+notext=
 
 # cross-compilation prefix
 CROSS=
@@ -353,6 +354,7 @@ epapr)
     platformo="$object/pseries-head.o $object/epapr.o $object/epapr-wrapper.o"
     link_address='0x20000000'
     pie=-pie
+    notext='-z notext'
     ;;
 mvme5100)
     platformo="$object/fixed-head.o $object/mvme5100.o"
@@ -493,8 +495,8 @@ if [ "$platform" != "miboot" ]; then
         text_start="-Ttext $link_address"
     fi
 #link everything
-    ${CROSS}ld -m $format -T $lds $text_start $pie $nodl -o "$ofile" $map \
-	$platformo $tmp $object/wrapper.a
+    ${CROSS}ld -m $format -T $lds $text_start $pie $notext $nodl -o "$ofile" \
+        $map $platformo $tmp $object/wrapper.a
     rm $tmp
 fi
 
-- 
2.29.0.rc1.297.gfa9743e501-goog

[PATCH 2/2] powerpc/boot: Use clang when CC is clang

[Bill Wendling]

The gcc compiler may not be available if CC is clang.

Cc: Fangrui Song <maskray@google.com>
Cc: Alan Modra <amodra@gmail.com>
Signed-off-by: Bill Wendling <morbo@google.com>
---
 arch/powerpc/boot/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index b88fd27a45f0..218f1c9adb5b 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -21,7 +21,11 @@
 all: $(obj)/zImage
 
 ifdef CROSS32_COMPILE
+ifdef CONFIG_CC_IS_CLANG
+    BOOTCC := $(CROSS32_COMPILE)clang
+else
     BOOTCC := $(CROSS32_COMPILE)gcc
+endif
     BOOTAR := $(CROSS32_COMPILE)ar
 else
     BOOTCC := $(CC)
-- 
2.29.0.rc1.297.gfa9743e501-goog

[PATCH 0/3] PPC: fixes for clang support

[Bill Wendling]

This series of patches include fixes for clang issues that arose. The
"powerpc/64s" patch was "inspired" by a similar patch for ARM:

eb7c11ee3c5ce arm64: alternative: Work around .inst assembler bugs

Bill Wendling (3):
  powerpc/wrapper: add "-z notext" flag to disable diagnostic
  powerpc/boot: Use clang when CC is clang
  powerpc/64s: feature: work around inline asm issues

 arch/powerpc/boot/Makefile                |  4 ++++
 arch/powerpc/boot/wrapper                 |  4 +++-
 arch/powerpc/include/asm/feature-fixups.h | 19 ++++++++++++++-----
 3 files changed, 21 insertions(+), 6 deletions(-)

-- 
2.29.2.454.gaff20da3a2-goog

[PATCH 1/3] powerpc/wrapper: add "-z notext" flag to disable diagnostic

[Bill Wendling]

The "-z notext" flag disables reporting an error if DT_TEXTREL is set.

  ld.lld: error: can't create dynamic relocation R_PPC64_ADDR64 against
    symbol: _start in readonly segment; recompile object files with
    -fPIC or pass '-Wl,-z,notext' to allow text relocations in the
    output
  >>> defined in
  >>> referenced by crt0.o:(.text+0x8) in archive arch/powerpc/boot/wrapper.a

The BFD linker disables this by default (though it's configurable in
current versions). LLD enables this by default. So we add the flag to
keep LLD from emitting the error.

Cc: Fangrui Song <maskray@google.com>
Cc: Alan Modra <amodra@gmail.com>
Signed-off-by: Bill Wendling <morbo@google.com>
---
 arch/powerpc/boot/wrapper | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/boot/wrapper b/arch/powerpc/boot/wrapper
index e1194955adbb..41fa0a8715e3 100755
--- a/arch/powerpc/boot/wrapper
+++ b/arch/powerpc/boot/wrapper
@@ -46,6 +46,7 @@ compression=.gz
 uboot_comp=gzip
 pie=
 format=
+notext=
 rodynamic=
 
 # cross-compilation prefix
@@ -354,6 +355,7 @@ epapr)
     platformo="$object/pseries-head.o $object/epapr.o $object/epapr-wrapper.o"
     link_address='0x20000000'
     pie=-pie
+    notext='-z notext'
     rodynamic=$(if ${CROSS}ld -V 2>&1 | grep -q LLD ; then echo "-z rodynamic"; fi)
     ;;
 mvme5100)
@@ -495,7 +497,7 @@ if [ "$platform" != "miboot" ]; then
         text_start="-Ttext $link_address"
     fi
 #link everything
-    ${CROSS}ld -m $format -T $lds $text_start $pie $nodl $rodynamic -o "$ofile" $map \
+    ${CROSS}ld -m $format -T $lds $text_start $pie $nodl $rodynamic $notext -o "$ofile" $map \
 	$platformo $tmp $object/wrapper.a
     rm $tmp
 fi
-- 
2.29.2.454.gaff20da3a2-goog

[PATCH 2/3] powerpc/boot: Use clang when CC is clang

[Bill Wendling]

The gcc compiler may not be available if CC is clang.

Signed-off-by: Bill Wendling <morbo@google.com>
---
 arch/powerpc/boot/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index f8ce6d2dde7b..68a7534454cd 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -21,7 +21,11 @@
 all: $(obj)/zImage
 
 ifdef CROSS32_COMPILE
+ifdef CONFIG_CC_IS_CLANG
+    BOOTCC := $(CROSS32_COMPILE)clang
+else
     BOOTCC := $(CROSS32_COMPILE)gcc
+endif
     BOOTAR := $(CROSS32_COMPILE)ar
 else
     BOOTCC := $(CC)
-- 
2.29.2.454.gaff20da3a2-goog

[PATCH 3/3] powerpc/64s: feature: work around inline asm issues

[Bill Wendling]

The clang toolchain treats inline assembly a bit differently than
straight assembly code. In particular, inline assembly doesn't have the
complete context available to resolve expressions. This is intentional
to avoid divergence in the resulting assembly code.

We can work around this issue by borrowing a workaround done for ARM,
i.e. not directly testing the labels themselves, but by moving the
current output pointer by a value that should always be zero. If this
value is not null, then we will trigger a backward move, which is
explicitly forbidden.

Signed-off-by: Bill Wendling <morbo@google.com>
---
 arch/powerpc/include/asm/feature-fixups.h | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/feature-fixups.h b/arch/powerpc/include/asm/feature-fixups.h
index b0af97add751..34331c4ba61a 100644
--- a/arch/powerpc/include/asm/feature-fixups.h
+++ b/arch/powerpc/include/asm/feature-fixups.h
@@ -36,6 +36,18 @@ label##2:						\
 	.align 2;					\
 label##3:
 
+/*
+ * If the .org directive fails, it means that the feature instructions
+ * are smaller than the alternate instructions. This used to be written
+ * as
+ *
+ * .ifgt (label##4b-label##3b) - (label##2b-label##1b)
+ *      .error "Feature section else case larger than body"
+ * .endif
+ *
+ * but clang's assembler complains about the expression being non-absolute
+ * when the code appears in an inline assembly statement.
+ */
 #define MAKE_FTR_SECTION_ENTRY(msk, val, label, sect)		\
 label##4:							\
 	.popsection;						\
@@ -48,11 +60,8 @@ 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;							\
-	.popsection;
-
+	.popsection;						\
+	.org . - ((label##4b-label##3b) > (label##2b-label##1b));
 
 /* CPU feature dependent sections */
 #define BEGIN_FTR_SECTION_NESTED(label)	START_FTR_SECTION(label)
-- 
2.29.2.454.gaff20da3a2-goog

[PATCH v3 0/3] PPC: fixes for clang support

[Bill Wendling]

Note: This is a resend of PPC/clang patches I sent before. The previous series
had a bad title, and one of the patches had a typo in it.

This series of patches include fixes for clang issues that arose. The
"powerpc/64s" patch was "inspired" by a similar patch for ARM:

eb7c11ee3c5ce arm64: alternative: Work around .inst assembler bugs

Bill Wendling (3):
  powerpc/wrapper: add "-z notext" flag to disable diagnostic
  powerpc/boot: Use clang when CC is clang
  powerpc/64s: feature: Work around inline asm issues

 arch/powerpc/boot/Makefile                |  4 ++++
 arch/powerpc/boot/wrapper                 |  4 +++-
 arch/powerpc/include/asm/feature-fixups.h | 17 +++++++++++++----
 3 files changed, 20 insertions(+), 5 deletions(-)

-- 
2.29.2.454.gaff20da3a2-goog

[PATCH 1/3] powerpc/wrapper: add "-z notext" flag to disable diagnostic

[Bill Wendling]

The "-z notext" flag disables reporting an error if DT_TEXTREL is set.

  ld.lld: error: can't create dynamic relocation R_PPC64_ADDR64 against
    symbol: _start in readonly segment; recompile object files with
    -fPIC or pass '-Wl,-z,notext' to allow text relocations in the
    output
  >>> defined in
  >>> referenced by crt0.o:(.text+0x8) in archive arch/powerpc/boot/wrapper.a

The BFD linker disables this by default (though it's configurable in
current versions). LLD enables this by default. So we add the flag to
keep LLD from emitting the error.

Cc: Fangrui Song <maskray@google.com>
Cc: Alan Modra <amodra@gmail.com>
Signed-off-by: Bill Wendling <morbo@google.com>
---
 arch/powerpc/boot/wrapper | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/boot/wrapper b/arch/powerpc/boot/wrapper
index e1194955adbb..41fa0a8715e3 100755
--- a/arch/powerpc/boot/wrapper
+++ b/arch/powerpc/boot/wrapper
@@ -46,6 +46,7 @@ compression=.gz
 uboot_comp=gzip
 pie=
 format=
+notext=
 rodynamic=
 
 # cross-compilation prefix
@@ -354,6 +355,7 @@ epapr)
     platformo="$object/pseries-head.o $object/epapr.o $object/epapr-wrapper.o"
     link_address='0x20000000'
     pie=-pie
+    notext='-z notext'
     rodynamic=$(if ${CROSS}ld -V 2>&1 | grep -q LLD ; then echo "-z rodynamic"; fi)
     ;;
 mvme5100)
@@ -495,7 +497,7 @@ if [ "$platform" != "miboot" ]; then
         text_start="-Ttext $link_address"
     fi
 #link everything
-    ${CROSS}ld -m $format -T $lds $text_start $pie $nodl $rodynamic -o "$ofile" $map \
+    ${CROSS}ld -m $format -T $lds $text_start $pie $nodl $rodynamic $notext -o "$ofile" $map \
 	$platformo $tmp $object/wrapper.a
     rm $tmp
 fi
-- 
2.29.2.454.gaff20da3a2-goog

[PATCH v3 2/3] powerpc/boot: Use clang when CC is clang

[Bill Wendling]

The gcc compiler may not be available if CC is clang.

Signed-off-by: Bill Wendling <morbo@google.com>
---
 arch/powerpc/boot/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index f8ce6d2dde7b..68a7534454cd 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -21,7 +21,11 @@
 all: $(obj)/zImage
 
 ifdef CROSS32_COMPILE
+ifdef CONFIG_CC_IS_CLANG
+    BOOTCC := $(CROSS32_COMPILE)clang
+else
     BOOTCC := $(CROSS32_COMPILE)gcc
+endif
     BOOTAR := $(CROSS32_COMPILE)ar
 else
     BOOTCC := $(CC)
-- 
2.29.2.454.gaff20da3a2-goog

[PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues

[Bill Wendling]

The clang toolchain treats inline assembly a bit differently than
straight assembly code. In particular, inline assembly doesn't have the
complete context available to resolve expressions. This is intentional
to avoid divergence in the resulting assembly code.

We can work around this issue by borrowing a workaround done for ARM,
i.e. not directly testing the labels themselves, but by moving the
current output pointer by a value that should always be zero. If this
value is not null, then we will trigger a backward move, which is
explicitly forbidden.

Signed-off-by: Bill Wendling <morbo@google.com>
---
 arch/powerpc/include/asm/feature-fixups.h | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/feature-fixups.h b/arch/powerpc/include/asm/feature-fixups.h
index b0af97add751..f81036518edb 100644
--- a/arch/powerpc/include/asm/feature-fixups.h
+++ b/arch/powerpc/include/asm/feature-fixups.h
@@ -36,6 +36,18 @@ label##2:						\
 	.align 2;					\
 label##3:
 
+/*
+ * If the .org directive fails, it means that the feature instructions
+ * are smaller than the alternate instructions. This used to be written
+ * as
+ *
+ * .ifgt (label##4b-label##3b) - (label##2b-label##1b)
+ *      .error "Feature section else case larger than body"
+ * .endif
+ *
+ * but clang's assembler complains about the expression being non-absolute
+ * when the code appears in an inline assembly statement.
+ */
 #define MAKE_FTR_SECTION_ENTRY(msk, val, label, sect)		\
 label##4:							\
 	.popsection;						\
@@ -48,12 +60,9 @@ 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;							\
+	.org . - ((label##4b-label##3b) > (label##2b-label##1b)); \
 	.popsection;
 
-
 /* CPU feature dependent sections */
 #define BEGIN_FTR_SECTION_NESTED(label)	START_FTR_SECTION(label)
 #define BEGIN_FTR_SECTION		START_FTR_SECTION(97)
-- 
2.29.2.454.gaff20da3a2-goog

Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues

[Michael Ellerman]

Hi Bill,

Bill Wendling <morbo@google.com> writes:
> The clang toolchain treats inline assembly a bit differently than
> straight assembly code. In particular, inline assembly doesn't have the
> complete context available to resolve expressions. This is intentional
> to avoid divergence in the resulting assembly code.
>
> We can work around this issue by borrowing a workaround done for ARM,
> i.e. not directly testing the labels themselves, but by moving the
> current output pointer by a value that should always be zero. If this
> value is not null, then we will trigger a backward move, which is
> explicitly forbidden.
>
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
>  arch/powerpc/include/asm/feature-fixups.h | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/feature-fixups.h b/arch/powerpc/include/asm/feature-fixups.h
> index b0af97add751..f81036518edb 100644
> --- a/arch/powerpc/include/asm/feature-fixups.h
> +++ b/arch/powerpc/include/asm/feature-fixups.h
> @@ -36,6 +36,18 @@ label##2:						\
>  	.align 2;					\
>  label##3:
>  
> +/*
> + * If the .org directive fails, it means that the feature instructions
> + * are smaller than the alternate instructions. This used to be written
> + * as
> + *
> + * .ifgt (label##4b-label##3b) - (label##2b-label##1b)
> + *      .error "Feature section else case larger than body"
> + * .endif
> + *
> + * but clang's assembler complains about the expression being non-absolute
> + * when the code appears in an inline assembly statement.
> + */
>  #define MAKE_FTR_SECTION_ENTRY(msk, val, label, sect)		\
>  label##4:							\
>  	.popsection;						\
> @@ -48,12 +60,9 @@ 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;							\
> +	.org . - ((label##4b-label##3b) > (label##2b-label##1b)); \
>  	.popsection;

When I have an oversize alt section this doesn't seem to give me any
error using binutils?

If I hard code:

	.org . - (1);

It fails as expected.

But if I hard code:

	.org . - (1 > 0);

It builds?

cheers

Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues

[Segher Boessenkool]

On Mon, Nov 23, 2020 at 04:44:56PM +1100, Michael Ellerman wrote:
> If I hard code:
> 
> 	.org . - (1);
> 
> It fails as expected.
> 
> But if I hard code:
> 
> 	.org . - (1 > 0);
> 
> It builds?

"true" (as a result of a comparison) in as is -1, not 1.


Segher

Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues

[Bill Wendling]

What Segher said. :-) Also, if you reverse the comparison, you'll get
a build error.

On Sun, Nov 22, 2020 at 10:36 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Mon, Nov 23, 2020 at 04:44:56PM +1100, Michael Ellerman wrote:
> > If I hard code:
> >
> >       .org . - (1);
> >
> > It fails as expected.
> >
> > But if I hard code:
> >
> >       .org . - (1 > 0);
> >
> > It builds?
>
> "true" (as a result of a comparison) in as is -1, not 1.
>
>
> Segher

Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues

[Bill Wendling]

After looking at this, I suspect that the correct change should be:

  .org . + ((label##4b-label##3b) > (label##2b-label##1b));

I'm sorry about that. I can submit another version of the patch.

-bw

On Mon, Nov 23, 2020 at 11:43 AM Bill Wendling <morbo@google.com> wrote:
>
> What Segher said. :-) Also, if you reverse the comparison, you'll get
> a build error.
>
> On Sun, Nov 22, 2020 at 10:36 PM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> >
> > On Mon, Nov 23, 2020 at 04:44:56PM +1100, Michael Ellerman wrote:
> > > If I hard code:
> > >
> > >       .org . - (1);
> > >
> > > It fails as expected.
> > >
> > > But if I hard code:
> > >
> > >       .org . - (1 > 0);
> > >
> > > It builds?
> >
> > "true" (as a result of a comparison) in as is -1, not 1.
> >
> >
> > Segher

Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues

[Segher Boessenkool]

(Please don't top-post.)

> On Sun, Nov 22, 2020 at 10:36 PM Segher Boessenkool
> <segher@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 -.  Was this tested?


Segher

Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues

[Bill Wendling]

On Mon, Nov 23, 2020 at 11:58 AM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> > On Sun, Nov 22, 2020 at 10:36 PM Segher Boessenkool
> > <segher@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.

> Was this tested?
>
Please don't be insulting. Anyone can make an error.

-bw

Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues

[Segher Boessenkool]

On Mon, Nov 23, 2020 at 12:01:01PM -0800, Bill Wendling wrote:
> On Mon, Nov 23, 2020 at 11:58 AM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> > > On Sun, Nov 22, 2020 at 10:36 PM Segher Boessenkool
> > > <segher@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" :-)

> > Was this tested?
> >
> Please don't be insulting. Anyone can make an error.

Absolutely, but it is just a question.  It seems you could improve that
testing!  It helps you yourself most of all ;-)


Segher

Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues

[Bill Wendling]

On Mon, Nov 23, 2020 at 12:10 PM Segher Boessenkool
<segher@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
> > <segher@kernel.crashing.org> wrote:
> > > > On Sun, Nov 22, 2020 at 10:36 PM Segher Boessenkool
> > > > <segher@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.

-bw

Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues

[Michael Ellerman]

Bill Wendling <morbo@google.com> writes:
> On Mon, Nov 23, 2020 at 12:10 PM Segher Boessenkool
> <segher@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
>> > <segher@kernel.crashing.org> wrote:
>> > > > On Sun, Nov 22, 2020 at 10:36 PM Segher Boessenkool
>> > > > <segher@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


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.

cheers

Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues

[Bill Wendling]

On Mon, Nov 23, 2020 at 7:44 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
> Bill Wendling <morbo@google.com> writes:
> > On Mon, Nov 23, 2020 at 12:10 PM Segher Boessenkool
> > <segher@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
> >> > <segher@kernel.crashing.org> wrote:
> >> > > > On Sun, Nov 22, 2020 at 10:36 PM Segher Boessenkool
> >> > > > <segher@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!

> 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.

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.


-bw

Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues

[Michael Ellerman]

Bill Wendling <morbo@google.com> writes:
> On Mon, Nov 23, 2020 at 7:44 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Bill Wendling <morbo@google.com> writes:
>> > On Mon, Nov 23, 2020 at 12:10 PM Segher Boessenkool
>> > <segher@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
>> >> > <segher@kernel.crashing.org> wrote:
>> >> > > > On Sun, Nov 22, 2020 at 10:36 PM Segher Boessenkool
>> >> > > > <segher@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

Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues

[Bill Wendling]

[-- Attachment #1: Type: text/plain, Size: 5310 bytes --]

On Thu, Nov 26, 2020, 5:03 PM Michael Ellerman <mpe@ellerman.id.au> wrote:

> Bill Wendling <morbo@google.com> writes:
> > On Mon, Nov 23, 2020 at 7:44 PM Michael Ellerman <mpe@ellerman.id.au>
> wrote:
> >> Bill Wendling <morbo@google.com> writes:
> >> > On Mon, Nov 23, 2020 at 12:10 PM Segher Boessenkool
> >> > <segher@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
> >> >> > <segher@kernel.crashing.org> wrote:
> >> >> > > > On Sun, Nov 22, 2020 at 10:36 PM Segher Boessenkool
> >> >> > > > <segher@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?
>

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.

> 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.
>

Thanks!
-bw

>

[-- Attachment #2: Type: text/html, Size: 8408 bytes --]

Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues

[Bill Wendling]

On Thu, Nov 26, 2020 at 5:03 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Bill Wendling <morbo@google.com> writes:
> > On Mon, Nov 23, 2020 at 7:44 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
> >> Bill Wendling <morbo@google.com> writes:
> >> > On Mon, Nov 23, 2020 at 12:10 PM Segher Boessenkool
> >> > <segher@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
> >> >> > <segher@kernel.crashing.org> wrote:
> >> >> > > > On Sun, Nov 22, 2020 at 10:36 PM Segher Boessenkool
> >> >> > > > <segher@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?
>
[Resent, because my previous email went out as non-plain text.]

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.

> > 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.
>
Thanks!

-bw