Messages in this thread Patch in this message | | | From | Arnd Bergmann <> | Date | Fri, 9 Aug 2019 22:12:56 +0200 | Subject | Re: [PATCH] powerpc: fix inline asm constraints for dcbz |
| |
On Fri, Aug 9, 2019 at 10:02 PM Christophe Leroy <christophe.leroy@c-s.fr> wrote: > > Arnd Bergmann <arnd@arndb.de> a écrit : > > On Fri, Aug 9, 2019 at 8:21 PM 'Nick Desaulniers' via Clang Built > > Linux <clang-built-linux@googlegroups.com> wrote: > > > >> static inline void dcbz(void *addr) > >> { > >> - __asm__ __volatile__ ("dcbz %y0" : : "Z"(*(u8 *)addr) : "memory"); > >> + __asm__ __volatile__ ("dcbz %y0" : "=Z"(*(u8 *)addr) :: "memory"); > >> } > >> > >> static inline void dcbi(void *addr) > >> { > >> - __asm__ __volatile__ ("dcbi %y0" : : "Z"(*(u8 *)addr) : "memory"); > >> + __asm__ __volatile__ ("dcbi %y0" : "=Z"(*(u8 *)addr) :: "memory"); > >> } > > > > I think the result of the discussion was that an output argument only kind-of > > makes sense for dcbz, but for the others it's really an input, and clang is > > wrong in the way it handles the "Z" constraint by making a copy, which it > > doesn't do for "m". > > > > I'm not sure whether it's correct to use "m" instead of "Z" here, which > > would be a better workaround if that works. More importantly though, > > clang really needs to be fixed to handle "Z" correctly. > > As the benefit is null, I think the best is probably to reverse my > original commit until at least CLang is fixed, as initialy suggested > by mpe
Yes, makes sense.
There is one other use of the "Z" constraint, so on top of the revert, I think it might be helpful if Nick could check if the patch below makes any difference with clang and, if it does, whether the current version is broken.
Arnd
diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h index 23e5d5d16c7e..28b467779328 100644 --- a/arch/powerpc/include/asm/io.h +++ b/arch/powerpc/include/asm/io.h @@ -106,7 +106,7 @@ static inline u##size name(const volatile u##size __iomem *addr) \ { \ u##size ret; \ __asm__ __volatile__("sync;"#insn" %0,%y1;twi 0,%0,0;isync" \ - : "=r" (ret) : "Z" (*addr) : "memory"); \ + : "=r" (ret) : "m" (*addr) : "memory"); \ return ret; \ }
@@ -114,7 +114,7 @@ static inline u##size name(const volatile u##size __iomem *addr) \ static inline void name(volatile u##size __iomem *addr, u##size val) \ { \ __asm__ __volatile__("sync;"#insn" %1,%y0" \ - : "=Z" (*addr) : "r" (val) : "memory"); \ + : "=m" (*addr) : "r" (val) : "memory"); \ mmiowb_set_pending(); \ }
| |