* [PATCH 0/2] Fixes for clang/lld @ 2020-10-17 0:47 Bill Wendling 2020-10-17 0:47 ` [PATCH 1/2] powerpc/wrapper: Add "-z notext" flag to disable diagnostic Bill Wendling 2020-10-17 0:47 ` [PATCH 2/2] powerpc/boot: Use clang when CC is clang Bill Wendling 0 siblings, 2 replies; 24+ messages in thread From: Bill Wendling @ 2020-10-17 0:47 UTC (permalink / raw) To: linuxppc-dev; +Cc: Paul Mackerras, 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/2] powerpc/wrapper: Add "-z notext" flag to disable diagnostic 2020-10-17 0:47 [PATCH 0/2] Fixes for clang/lld Bill Wendling @ 2020-10-17 0:47 ` Bill Wendling 2020-10-17 0:47 ` [PATCH 2/2] powerpc/boot: Use clang when CC is clang Bill Wendling 1 sibling, 0 replies; 24+ messages in thread From: Bill Wendling @ 2020-10-17 0:47 UTC (permalink / raw) To: linuxppc-dev; +Cc: Fangrui Song, Alan Modra, Paul Mackerras, 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/2] powerpc/boot: Use clang when CC is clang 2020-10-17 0:47 [PATCH 0/2] Fixes for clang/lld Bill Wendling 2020-10-17 0:47 ` [PATCH 1/2] powerpc/wrapper: Add "-z notext" flag to disable diagnostic Bill Wendling @ 2020-10-17 0:47 ` Bill Wendling 2020-11-18 22:35 ` [PATCH 0/3] PPC: fixes for clang support Bill Wendling ` (3 more replies) 1 sibling, 4 replies; 24+ messages in thread From: Bill Wendling @ 2020-10-17 0:47 UTC (permalink / raw) To: linuxppc-dev; +Cc: Fangrui Song, Alan Modra, Paul Mackerras, 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 0/3] PPC: fixes for clang support 2020-10-17 0:47 ` [PATCH 2/2] powerpc/boot: Use clang when CC is clang Bill Wendling @ 2020-11-18 22:35 ` Bill Wendling 2020-11-20 22:40 ` [PATCH v3 " Bill Wendling 2020-11-18 22:35 ` [PATCH 1/3] powerpc/wrapper: add "-z notext" flag to disable diagnostic Bill Wendling ` (2 subsequent siblings) 3 siblings, 1 reply; 24+ messages in thread From: Bill Wendling @ 2020-11-18 22:35 UTC (permalink / raw) To: Michael Ellerman, linuxppc-dev; +Cc: Nick Desaulniers, 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 0/3] PPC: fixes for clang support 2020-11-18 22:35 ` [PATCH 0/3] PPC: fixes for clang support Bill Wendling @ 2020-11-20 22:40 ` Bill Wendling 2020-11-20 22:40 ` [PATCH 1/3] powerpc/wrapper: add "-z notext" flag to disable diagnostic Bill Wendling ` (2 more replies) 0 siblings, 3 replies; 24+ messages in thread From: Bill Wendling @ 2020-11-20 22:40 UTC (permalink / raw) To: Michael Ellerman, linuxppc-dev; +Cc: Nick Desaulniers, 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/3] powerpc/wrapper: add "-z notext" flag to disable diagnostic 2020-11-20 22:40 ` [PATCH v3 " Bill Wendling @ 2020-11-20 22:40 ` Bill Wendling 2020-11-20 22:40 ` [PATCH v3 2/3] powerpc/boot: Use clang when CC is clang Bill Wendling 2020-11-20 22:40 ` [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues Bill Wendling 2 siblings, 0 replies; 24+ messages in thread From: Bill Wendling @ 2020-11-20 22:40 UTC (permalink / raw) To: Michael Ellerman, linuxppc-dev Cc: Nick Desaulniers, Bill Wendling, Fangrui Song, Alan Modra 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 2/3] powerpc/boot: Use clang when CC is clang 2020-11-20 22:40 ` [PATCH v3 " Bill Wendling 2020-11-20 22:40 ` [PATCH 1/3] powerpc/wrapper: add "-z notext" flag to disable diagnostic Bill Wendling @ 2020-11-20 22:40 ` Bill Wendling 2020-11-20 22:40 ` [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues Bill Wendling 2 siblings, 0 replies; 24+ messages in thread From: Bill Wendling @ 2020-11-20 22:40 UTC (permalink / raw) To: Michael Ellerman, linuxppc-dev; +Cc: Nick Desaulniers, 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues 2020-11-20 22:40 ` [PATCH v3 " Bill Wendling 2020-11-20 22:40 ` [PATCH 1/3] powerpc/wrapper: add "-z notext" flag to disable diagnostic Bill Wendling 2020-11-20 22:40 ` [PATCH v3 2/3] powerpc/boot: Use clang when CC is clang Bill Wendling @ 2020-11-20 22:40 ` Bill Wendling 2020-11-23 5:44 ` Michael Ellerman 2 siblings, 1 reply; 24+ messages in thread From: Bill Wendling @ 2020-11-20 22:40 UTC (permalink / raw) To: Michael Ellerman, linuxppc-dev; +Cc: Nick Desaulniers, 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues 2020-11-20 22:40 ` [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues Bill Wendling @ 2020-11-23 5:44 ` Michael Ellerman 2020-11-23 6:34 ` Segher Boessenkool 0 siblings, 1 reply; 24+ messages in thread From: Michael Ellerman @ 2020-11-23 5:44 UTC (permalink / raw) To: Bill Wendling, linuxppc-dev; +Cc: Nick Desaulniers, Bill Wendling 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues 2020-11-23 5:44 ` Michael Ellerman @ 2020-11-23 6:34 ` Segher Boessenkool 2020-11-23 19:43 ` Bill Wendling 0 siblings, 1 reply; 24+ messages in thread From: Segher Boessenkool @ 2020-11-23 6:34 UTC (permalink / raw) To: Michael Ellerman; +Cc: Nick Desaulniers, linuxppc-dev, Bill Wendling 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues 2020-11-23 6:34 ` Segher Boessenkool @ 2020-11-23 19:43 ` Bill Wendling 2020-11-23 19:53 ` Bill Wendling 2020-11-23 19:56 ` Segher Boessenkool 0 siblings, 2 replies; 24+ messages in thread From: Bill Wendling @ 2020-11-23 19:43 UTC (permalink / raw) To: Segher Boessenkool; +Cc: Nick Desaulniers, linuxppc-dev 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues 2020-11-23 19:43 ` Bill Wendling @ 2020-11-23 19:53 ` Bill Wendling 2020-11-23 19:56 ` Segher Boessenkool 1 sibling, 0 replies; 24+ messages in thread From: Bill Wendling @ 2020-11-23 19:53 UTC (permalink / raw) To: Segher Boessenkool; +Cc: Nick Desaulniers, linuxppc-dev 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues 2020-11-23 19:43 ` Bill Wendling 2020-11-23 19:53 ` Bill Wendling @ 2020-11-23 19:56 ` Segher Boessenkool 2020-11-23 20:01 ` Bill Wendling 1 sibling, 1 reply; 24+ messages in thread From: Segher Boessenkool @ 2020-11-23 19:56 UTC (permalink / raw) To: Bill Wendling; +Cc: Nick Desaulniers, linuxppc-dev (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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues 2020-11-23 19:56 ` Segher Boessenkool @ 2020-11-23 20:01 ` Bill Wendling 2020-11-23 20:08 ` Segher Boessenkool 0 siblings, 1 reply; 24+ messages in thread From: Bill Wendling @ 2020-11-23 20:01 UTC (permalink / raw) To: Segher Boessenkool; +Cc: Nick Desaulniers, linuxppc-dev 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues 2020-11-23 20:01 ` Bill Wendling @ 2020-11-23 20:08 ` Segher Boessenkool 2020-11-23 20:17 ` Bill Wendling 0 siblings, 1 reply; 24+ messages in thread From: Segher Boessenkool @ 2020-11-23 20:08 UTC (permalink / raw) To: Bill Wendling; +Cc: Nick Desaulniers, linuxppc-dev 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues 2020-11-23 20:08 ` Segher Boessenkool @ 2020-11-23 20:17 ` Bill Wendling 2020-11-24 3:43 ` Michael Ellerman 0 siblings, 1 reply; 24+ messages in thread From: Bill Wendling @ 2020-11-23 20:17 UTC (permalink / raw) To: Segher Boessenkool; +Cc: Nick Desaulniers, linuxppc-dev 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues 2020-11-23 20:17 ` Bill Wendling @ 2020-11-24 3:43 ` Michael Ellerman 2020-11-25 5:13 ` Bill Wendling 0 siblings, 1 reply; 24+ messages in thread From: Michael Ellerman @ 2020-11-24 3:43 UTC (permalink / raw) To: Bill Wendling, Segher Boessenkool; +Cc: Nick Desaulniers, linuxppc-dev 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues 2020-11-24 3:43 ` Michael Ellerman @ 2020-11-25 5:13 ` Bill Wendling 2020-11-27 1:03 ` Michael Ellerman 0 siblings, 1 reply; 24+ messages in thread From: Bill Wendling @ 2020-11-25 5:13 UTC (permalink / raw) To: Michael Ellerman; +Cc: Nick Desaulniers, linuxppc-dev 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues 2020-11-25 5:13 ` Bill Wendling @ 2020-11-27 1:03 ` Michael Ellerman 2020-11-27 1:10 ` Bill Wendling 2020-11-27 1:59 ` Bill Wendling 0 siblings, 2 replies; 24+ messages in thread From: Michael Ellerman @ 2020-11-27 1:03 UTC (permalink / raw) To: Bill Wendling; +Cc: Nick Desaulniers, linuxppc-dev 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues 2020-11-27 1:03 ` Michael Ellerman @ 2020-11-27 1:10 ` Bill Wendling 2020-11-27 1:59 ` Bill Wendling 1 sibling, 0 replies; 24+ messages in thread From: Bill Wendling @ 2020-11-27 1:10 UTC (permalink / raw) To: Michael Ellerman; +Cc: Nick Desaulniers, linuxppc-dev [-- 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 --] ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues 2020-11-27 1:03 ` Michael Ellerman 2020-11-27 1:10 ` Bill Wendling @ 2020-11-27 1:59 ` Bill Wendling 1 sibling, 0 replies; 24+ messages in thread From: Bill Wendling @ 2020-11-27 1:59 UTC (permalink / raw) To: Michael Ellerman; +Cc: Nick Desaulniers, linuxppc-dev 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/3] powerpc/wrapper: add "-z notext" flag to disable diagnostic 2020-10-17 0:47 ` [PATCH 2/2] powerpc/boot: Use clang when CC is clang Bill Wendling 2020-11-18 22:35 ` [PATCH 0/3] PPC: fixes for clang support Bill Wendling @ 2020-11-18 22:35 ` Bill Wendling 2020-11-18 22:35 ` [PATCH 2/3] powerpc/boot: Use clang when CC is clang Bill Wendling 2020-11-18 22:35 ` [PATCH 3/3] powerpc/64s: feature: work around inline asm issues Bill Wendling 3 siblings, 0 replies; 24+ messages in thread From: Bill Wendling @ 2020-11-18 22:35 UTC (permalink / raw) To: Michael Ellerman, linuxppc-dev Cc: Nick Desaulniers, Bill Wendling, Fangrui Song, Alan Modra 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/3] powerpc/boot: Use clang when CC is clang 2020-10-17 0:47 ` [PATCH 2/2] powerpc/boot: Use clang when CC is clang Bill Wendling 2020-11-18 22:35 ` [PATCH 0/3] PPC: fixes for clang support Bill Wendling 2020-11-18 22:35 ` [PATCH 1/3] powerpc/wrapper: add "-z notext" flag to disable diagnostic Bill Wendling @ 2020-11-18 22:35 ` Bill Wendling 2020-11-18 22:35 ` [PATCH 3/3] powerpc/64s: feature: work around inline asm issues Bill Wendling 3 siblings, 0 replies; 24+ messages in thread From: Bill Wendling @ 2020-11-18 22:35 UTC (permalink / raw) To: Michael Ellerman, linuxppc-dev; +Cc: Nick Desaulniers, 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/3] powerpc/64s: feature: work around inline asm issues 2020-10-17 0:47 ` [PATCH 2/2] powerpc/boot: Use clang when CC is clang Bill Wendling ` (2 preceding siblings ...) 2020-11-18 22:35 ` [PATCH 2/3] powerpc/boot: Use clang when CC is clang Bill Wendling @ 2020-11-18 22:35 ` Bill Wendling 3 siblings, 0 replies; 24+ messages in thread From: Bill Wendling @ 2020-11-18 22:35 UTC (permalink / raw) To: Michael Ellerman, linuxppc-dev; +Cc: Nick Desaulniers, 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 ^ permalink raw reply related [flat|nested] 24+ messages in thread
end of thread, other threads:[~2020-11-27 3:14 UTC | newest] Thread overview: 24+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-10-17 0:47 [PATCH 0/2] Fixes for clang/lld Bill Wendling 2020-10-17 0:47 ` [PATCH 1/2] powerpc/wrapper: Add "-z notext" flag to disable diagnostic Bill Wendling 2020-10-17 0:47 ` [PATCH 2/2] powerpc/boot: Use clang when CC is clang Bill Wendling 2020-11-18 22:35 ` [PATCH 0/3] PPC: fixes for clang support Bill Wendling 2020-11-20 22:40 ` [PATCH v3 " Bill Wendling 2020-11-20 22:40 ` [PATCH 1/3] powerpc/wrapper: add "-z notext" flag to disable diagnostic Bill Wendling 2020-11-20 22:40 ` [PATCH v3 2/3] powerpc/boot: Use clang when CC is clang Bill Wendling 2020-11-20 22:40 ` [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues Bill Wendling 2020-11-23 5:44 ` Michael Ellerman 2020-11-23 6:34 ` Segher Boessenkool 2020-11-23 19:43 ` Bill Wendling 2020-11-23 19:53 ` Bill Wendling 2020-11-23 19:56 ` Segher Boessenkool 2020-11-23 20:01 ` Bill Wendling 2020-11-23 20:08 ` Segher Boessenkool 2020-11-23 20:17 ` Bill Wendling 2020-11-24 3:43 ` Michael Ellerman 2020-11-25 5:13 ` Bill Wendling 2020-11-27 1:03 ` Michael Ellerman 2020-11-27 1:10 ` Bill Wendling 2020-11-27 1:59 ` Bill Wendling 2020-11-18 22:35 ` [PATCH 1/3] powerpc/wrapper: add "-z notext" flag to disable diagnostic Bill Wendling 2020-11-18 22:35 ` [PATCH 2/3] powerpc/boot: Use clang when CC is clang Bill Wendling 2020-11-18 22:35 ` [PATCH 3/3] powerpc/64s: feature: work around inline asm issues Bill Wendling
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.