| Message ID | 20200227213450.87194-4-palmer@dabbelt.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] RISC-V: Stop relying on GCC's register allocator's hueristics | expand |
On Thu, Feb 27, 2020 at 1:35 PM Palmer Dabbelt <palmer@dabbelt.com> wrote: > > From: Palmer Dabbelt <palmerdabbelt@google.com> > > LLVM's integrated assembler doesn't support the LOCAL directive, which we're > using when generating our uaccess fixup tables. Luckily the table fragment is > small enough that there's only one internal symbol, so using a relative symbol > reference doesn't really complicate anything. Is `LOCAL` a macro for `.local`? (Looks like no). I would think `.local` is supported, but `LOCAL` isn't something I've seen before. Ah, looks like it's local to macros: https://sourceware.org/binutils/docs/as/Macro.html#Macro "Warning: LOCAL is only available if you select “alternate macro syntax” with ‘--alternate’ or .altmacro. See .altmacro." https://sourceware.org/binutils/docs/as/Altmacro.html#Altmacro Link: https://sourceware.org/binutils/docs/as/Local.html#Local But these macros are setting .altmacro... So it looks like Clang's integrated assembler doesn't yet support `LOCAL`. Filed: https://bugs.llvm.org/show_bug.cgi?id=45051 If we're no longer using LOCAL, do we still need `.altmacro`? I also see two usages in: arch/riscv/kernel/entry.S Would you mind fixing those up, too? Otherwise patch LGTM. > > Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com> > --- > arch/riscv/lib/uaccess.S | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S > index f29d2ba2c0a6..40bf130073e8 100644 > --- a/arch/riscv/lib/uaccess.S > +++ b/arch/riscv/lib/uaccess.S > @@ -5,12 +5,11 @@ > > .altmacro > .macro fixup op reg addr lbl > - LOCAL _epc > -_epc: > +100: > \op \reg, \addr > .section __ex_table,"a" > .balign RISCV_SZPTR > - RISCV_PTR _epc, \lbl > + RISCV_PTR 100b, \lbl > .previous > .endm > > --
On Thu, Feb 27, 2020 at 3:03 PM Nick Desaulniers <ndesaulniers@google.com> wrote: > I also see two usages in: > arch/riscv/kernel/entry.S > > Would you mind fixing those up, too? Ah, removed by patch 2/3, sorry for reviewing them out of order! Outstanding question about `.altmacro` though.
On Thu, 27 Feb 2020 15:03:42 PST (-0800), Nick Desaulniers wrote: > On Thu, Feb 27, 2020 at 1:35 PM Palmer Dabbelt <palmer@dabbelt.com> wrote: >> >> From: Palmer Dabbelt <palmerdabbelt@google.com> >> >> LLVM's integrated assembler doesn't support the LOCAL directive, which we're >> using when generating our uaccess fixup tables. Luckily the table fragment is >> small enough that there's only one internal symbol, so using a relative symbol >> reference doesn't really complicate anything. > > Is `LOCAL` a macro for `.local`? (Looks like no). I would think > `.local` is supported, but `LOCAL` isn't something I've seen before. > > Ah, looks like it's local to macros: > https://sourceware.org/binutils/docs/as/Macro.html#Macro > "Warning: LOCAL is only available if you select “alternate macro > syntax” with ‘--alternate’ or .altmacro. See .altmacro." > https://sourceware.org/binutils/docs/as/Altmacro.html#Altmacro > Link: https://sourceware.org/binutils/docs/as/Local.html#Local > > But these macros are setting .altmacro... > So it looks like Clang's integrated assembler doesn't yet support > `LOCAL`. Filed: > https://bugs.llvm.org/show_bug.cgi?id=45051 > > If we're no longer using LOCAL, do we still need `.altmacro`? > > I also see two usages in: > arch/riscv/kernel/entry.S > > Would you mind fixing those up, too? Done. > > Otherwise patch LGTM. > >> >> Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com> >> --- >> arch/riscv/lib/uaccess.S | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S >> index f29d2ba2c0a6..40bf130073e8 100644 >> --- a/arch/riscv/lib/uaccess.S >> +++ b/arch/riscv/lib/uaccess.S >> @@ -5,12 +5,11 @@ >> >> .altmacro >> .macro fixup op reg addr lbl >> - LOCAL _epc >> -_epc: >> +100: >> \op \reg, \addr >> .section __ex_table,"a" >> .balign RISCV_SZPTR >> - RISCV_PTR _epc, \lbl >> + RISCV_PTR 100b, \lbl >> .previous >> .endm >> >> -- > > -- > Thanks, > ~Nick Desaulniers
diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S index f29d2ba2c0a6..40bf130073e8 100644 --- a/arch/riscv/lib/uaccess.S +++ b/arch/riscv/lib/uaccess.S @@ -5,12 +5,11 @@ .altmacro .macro fixup op reg addr lbl - LOCAL _epc -_epc: +100: \op \reg, \addr .section __ex_table,"a" .balign RISCV_SZPTR - RISCV_PTR _epc, \lbl + RISCV_PTR 100b, \lbl .previous .endm