* [PATCH] arm64: io: specify asm operand width for __iormb()
@ 2018-11-29 4:19 Nick Desaulniers
2018-11-29 9:03 ` Julien Thierry
0 siblings, 1 reply; 8+ messages in thread
From: Nick Desaulniers @ 2018-11-29 4:19 UTC (permalink / raw)
To: linux-arm-kernel
Fixes the warning produced from Clang:
./include/asm-generic/io.h:711:9: warning: value size does not match
register size specified by the constraint and modifier
[-Wasm-operand-widths]
return readl(addr);
^
./arch/arm64/include/asm/io.h:149:58: note: expanded from macro 'readl'
^
./include/asm-generic/io.h:711:9: note: use constraint modifier "w"
./arch/arm64/include/asm/io.h:149:50: note: expanded from macro 'readl'
^
./arch/arm64/include/asm/io.h:118:25: note: expanded from macro '__iormb'
asm volatile("eor %w0, %1, %1\n" \
^
Though we disable Clang's integrated assembler with -no-integrated-as,
it still tries to do some validation of assembler constraints.
While __iormb() is type agnostic to operand widths for argument v, its
lone use is to zero'd out via eor (exclusive or).
Fixes commit 6460d3201471 ("arm64: io: Ensure calls to delay routines
are ordered against prior readX()")
Link: https://github.com/ClangBuiltLinux/continuous-integration/issues/78
Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com>
---
Side note: is it not correct to cite SHAs from linux-next in "Fixes
commit ..." lines? I guess we can drop it.
Link to regression build:
https://travis-ci.com/ClangBuiltLinux/continuous-integration/builds/92799938
Link to build w/ this patch:
https://travis-ci.com/ClangBuiltLinux/continuous-integration/builds/92935901
arch/arm64/include/asm/io.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
index d42d00d8d5b6..dbdebf81162b 100644
--- a/arch/arm64/include/asm/io.h
+++ b/arch/arm64/include/asm/io.h
@@ -115,7 +115,7 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
* later instructions. This ensures that a subsequent call to \
* udelay() will be ordered due to the ISB in get_cycles(). \
*/ \
- asm volatile("eor %0, %1, %1\n" \
+ asm volatile("eor %0, %x1, %x1\n" \
"cbnz %0, ." \
: "=r" (tmp) : "r" (v) : "memory"); \
})
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH] arm64: io: specify asm operand width for __iormb() 2018-11-29 4:19 [PATCH] arm64: io: specify asm operand width for __iormb() Nick Desaulniers @ 2018-11-29 9:03 ` Julien Thierry 2018-11-29 10:49 ` Will Deacon 0 siblings, 1 reply; 8+ messages in thread From: Julien Thierry @ 2018-11-29 9:03 UTC (permalink / raw) To: linux-arm-kernel On 29/11/18 04:19, Nick Desaulniers wrote: > Fixes the warning produced from Clang: > ./include/asm-generic/io.h:711:9: warning: value size does not match > register size specified by the constraint and modifier > [-Wasm-operand-widths] > return readl(addr); > ^ > ./arch/arm64/include/asm/io.h:149:58: note: expanded from macro 'readl' > ^ > ./include/asm-generic/io.h:711:9: note: use constraint modifier "w" > ./arch/arm64/include/asm/io.h:149:50: note: expanded from macro 'readl' > ^ > ./arch/arm64/include/asm/io.h:118:25: note: expanded from macro '__iormb' > asm volatile("eor %w0, %1, %1\n" \ > ^ Why does the "eor %0, %1, %1" become "eor %w0, %1, %1" ? The variable passed to the inline assembly for %0 is unsigned long, so always 64-bits wide on arm64. Why is clang trying to use a 32-bit register for it? Although it's not really important since all this is just introducing a control dependency, I find it a bit odd. Thanks, > Though we disable Clang's integrated assembler with -no-integrated-as, > it still tries to do some validation of assembler constraints. > > While __iormb() is type agnostic to operand widths for argument v, its > lone use is to zero'd out via eor (exclusive or). > > Fixes commit 6460d3201471 ("arm64: io: Ensure calls to delay routines > are ordered against prior readX()") > Link: https://github.com/ClangBuiltLinux/continuous-integration/issues/78 > Suggested-by: Nathan Chancellor <natechancellor@gmail.com> > Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> > Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com> > --- > Side note: is it not correct to cite SHAs from linux-next in "Fixes > commit ..." lines? I guess we can drop it. > > Link to regression build: > https://travis-ci.com/ClangBuiltLinux/continuous-integration/builds/92799938 > > Link to build w/ this patch: > https://travis-ci.com/ClangBuiltLinux/continuous-integration/builds/92935901 > > > arch/arm64/include/asm/io.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h > index d42d00d8d5b6..dbdebf81162b 100644 > --- a/arch/arm64/include/asm/io.h > +++ b/arch/arm64/include/asm/io.h > @@ -115,7 +115,7 @@ static inline u64 __raw_readq(const volatile void __iomem *addr) > * later instructions. This ensures that a subsequent call to\ > * udelay() will be ordered due to the ISB in get_cycles().\ > */\ > -asm volatile("eor%0, %1, %1\n"\ > +asm volatile("eor%0, %x1, %x1\n"\ > "cbnz%0, ."\ > : "=r" (tmp) : "r" (v) : "memory");\ > }) > -- Julien Thierry IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] arm64: io: specify asm operand width for __iormb() 2018-11-29 9:03 ` Julien Thierry @ 2018-11-29 10:49 ` Will Deacon 2018-11-29 16:10 ` Nathan Chancellor 0 siblings, 1 reply; 8+ messages in thread From: Will Deacon @ 2018-11-29 10:49 UTC (permalink / raw) To: linux-arm-kernel On Thu, Nov 29, 2018 at 09:03:54AM +0000, Julien Thierry wrote: > > > On 29/11/18 04:19, Nick Desaulniers wrote: > > Fixes the warning produced from Clang: > > ./include/asm-generic/io.h:711:9: warning: value size does not match > > register size specified by the constraint and modifier > > [-Wasm-operand-widths] > > return readl(addr); > > ^ > > ./arch/arm64/include/asm/io.h:149:58: note: expanded from macro 'readl' > > ^ > > ./include/asm-generic/io.h:711:9: note: use constraint modifier "w" > > ./arch/arm64/include/asm/io.h:149:50: note: expanded from macro 'readl' > > ^ > > ./arch/arm64/include/asm/io.h:118:25: note: expanded from macro '__iormb' > > asm volatile("eor %w0, %1, %1\n" \ > > ^ > > Why does the "eor %0, %1, %1" become "eor %w0, %1, %1" ? > The variable passed to the inline assembly for %0 is unsigned long, so > always 64-bits wide on arm64. Why is clang trying to use a 32-bit > register for it? Yeah, the message above looks bogus to me. I can see %1 being 32-bit for read[bwl], so maybe clang is just getting the diagnostic wrong. If so, I wonder if the following fixes the problem: diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h index d42d00d8d5b6..13befec8b64e 100644 --- a/arch/arm64/include/asm/io.h +++ b/arch/arm64/include/asm/io.h @@ -117,7 +117,7 @@ static inline u64 __raw_readq(const volatile void __iomem *addr) */ \ asm volatile("eor %0, %1, %1\n" \ "cbnz %0, ." \ - : "=r" (tmp) : "r" (v) : "memory"); \ + : "=r" (tmp) : "r" (unsigned long)(v) : "memory"); \ }) #define __iowmb() wmb() Will ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: io: specify asm operand width for __iormb() 2018-11-29 10:49 ` Will Deacon @ 2018-11-29 16:10 ` Nathan Chancellor 2018-11-29 16:13 ` Will Deacon 2018-11-29 16:14 ` Russell King - ARM Linux 0 siblings, 2 replies; 8+ messages in thread From: Nathan Chancellor @ 2018-11-29 16:10 UTC (permalink / raw) To: Will Deacon Cc: Jens Axboe, Julien Thierry, Catalin Marinas, linux-kernel@vger.kernel.org, Nick Desaulniers, linux-arm-kernel@lists.infradead.org On Thu, Nov 29, 2018 at 10:49:03AM +0000, Will Deacon wrote: > On Thu, Nov 29, 2018 at 09:03:54AM +0000, Julien Thierry wrote: > > > > > > On 29/11/18 04:19, Nick Desaulniers wrote: > > > Fixes the warning produced from Clang: > > > ./include/asm-generic/io.h:711:9: warning: value size does not match > > > register size specified by the constraint and modifier > > > [-Wasm-operand-widths] > > > return readl(addr); > > > ^ > > > ./arch/arm64/include/asm/io.h:149:58: note: expanded from macro 'readl' > > > ^ > > > ./include/asm-generic/io.h:711:9: note: use constraint modifier "w" > > > ./arch/arm64/include/asm/io.h:149:50: note: expanded from macro 'readl' > > > ^ > > > ./arch/arm64/include/asm/io.h:118:25: note: expanded from macro '__iormb' > > > asm volatile("eor %w0, %1, %1\n" \ > > > ^ > > > > Why does the "eor %0, %1, %1" become "eor %w0, %1, %1" ? > > The variable passed to the inline assembly for %0 is unsigned long, so > > always 64-bits wide on arm64. Why is clang trying to use a 32-bit > > register for it? Sorry, this was my fault, I accidentally added a w during testing to see what constraints were valid (given that my assembly knowledge is nearly non-existence so forgive the non-sensical experimentation) and I used that message rather than the original one. This is the unadulterated one. In file included from arch/arm64/kernel/asm-offsets.c:24: In file included from ./include/linux/dma-mapping.h:11: In file included from ./include/linux/scatterlist.h:9: In file included from ./arch/arm64/include/asm/io.h:209: ./include/asm-generic/io.h:695:9: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths] return readb(addr); ^ ./arch/arm64/include/asm/io.h:147:58: note: expanded from macro 'readb' #define readb(c) ({ u8 __v = readb_relaxed(c); __iormb(__v); __v; }) ^ ./include/asm-generic/io.h:695:9: note: use constraint modifier "w" ./arch/arm64/include/asm/io.h:147:50: note: expanded from macro 'readb' #define readb(c) ({ u8 __v = readb_relaxed(c); __iormb(__v); __v; }) ^ ./arch/arm64/include/asm/io.h:118:24: note: expanded from macro '__iormb' asm volatile("eor %0, %1, %1\n" \ ^ > > Yeah, the message above looks bogus to me. I can see %1 being 32-bit for > read[bwl], so maybe clang is just getting the diagnostic wrong. If so, > I wonder if the following fixes the problem: > This doesn't appear to work, I get this error: In file included from arch/arm64/kernel/asm-offsets.c:24: In file included from ./include/linux/dma-mapping.h:11: In file included from ./include/linux/scatterlist.h:9: In file included from ./arch/arm64/include/asm/io.h:209: ./include/asm-generic/io.h:695:9: error: expected expression return readb(addr); ^ ./arch/arm64/include/asm/io.h:147:50: note: expanded from macro 'readb' #define readb(c) ({ u8 __v = readb_relaxed(c); __iormb(__v); __v; }) ^ ./arch/arm64/include/asm/io.h:120:28: note: expanded from macro '__iormb' : "=r" (tmp) : "r" (unsigned long)(v) : "memory"); \ ^ > > diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h > index d42d00d8d5b6..13befec8b64e 100644 > --- a/arch/arm64/include/asm/io.h > +++ b/arch/arm64/include/asm/io.h > @@ -117,7 +117,7 @@ static inline u64 __raw_readq(const volatile void __iomem *addr) > */ \ > asm volatile("eor %0, %1, %1\n" \ > "cbnz %0, ." \ > - : "=r" (tmp) : "r" (v) : "memory"); \ > + : "=r" (tmp) : "r" (unsigned long)(v) : "memory"); \ > }) > > #define __iowmb() wmb() > > > Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: io: specify asm operand width for __iormb() 2018-11-29 16:10 ` Nathan Chancellor @ 2018-11-29 16:13 ` Will Deacon 2018-11-29 16:17 ` Nathan Chancellor 2018-11-29 16:14 ` Russell King - ARM Linux 1 sibling, 1 reply; 8+ messages in thread From: Will Deacon @ 2018-11-29 16:13 UTC (permalink / raw) To: Nathan Chancellor Cc: Jens Axboe, Julien Thierry, Catalin Marinas, linux-kernel@vger.kernel.org, Nick Desaulniers, linux-arm-kernel@lists.infradead.org On Thu, Nov 29, 2018 at 09:10:39AM -0700, Nathan Chancellor wrote: > On Thu, Nov 29, 2018 at 10:49:03AM +0000, Will Deacon wrote: > > On Thu, Nov 29, 2018 at 09:03:54AM +0000, Julien Thierry wrote: > > > On 29/11/18 04:19, Nick Desaulniers wrote: > > > > Fixes the warning produced from Clang: > > > > ./include/asm-generic/io.h:711:9: warning: value size does not match > > > > register size specified by the constraint and modifier > > > > [-Wasm-operand-widths] > > > > return readl(addr); > > > > ^ > > > > ./arch/arm64/include/asm/io.h:149:58: note: expanded from macro 'readl' > > > > ^ > > > > ./include/asm-generic/io.h:711:9: note: use constraint modifier "w" > > > > ./arch/arm64/include/asm/io.h:149:50: note: expanded from macro 'readl' > > > > ^ > > > > ./arch/arm64/include/asm/io.h:118:25: note: expanded from macro '__iormb' > > > > asm volatile("eor %w0, %1, %1\n" \ > > > > ^ > > > > > > Why does the "eor %0, %1, %1" become "eor %w0, %1, %1" ? > > > The variable passed to the inline assembly for %0 is unsigned long, so > > > always 64-bits wide on arm64. Why is clang trying to use a 32-bit > > > register for it? > > Sorry, this was my fault, I accidentally added a w during testing to see > what constraints were valid (given that my assembly knowledge is nearly > non-existence so forgive the non-sensical experimentation) and I used > that message rather than the original one. This is the unadulterated one. Aha, that explains it. Thanks for clearing that up. > In file included from arch/arm64/kernel/asm-offsets.c:24: > In file included from ./include/linux/dma-mapping.h:11: > In file included from ./include/linux/scatterlist.h:9: > In file included from ./arch/arm64/include/asm/io.h:209: > ./include/asm-generic/io.h:695:9: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths] > return readb(addr); > ^ > ./arch/arm64/include/asm/io.h:147:58: note: expanded from macro 'readb' > #define readb(c) ({ u8 __v = readb_relaxed(c); __iormb(__v); __v; }) > ^ > ./include/asm-generic/io.h:695:9: note: use constraint modifier "w" > ./arch/arm64/include/asm/io.h:147:50: note: expanded from macro 'readb' > #define readb(c) ({ u8 __v = readb_relaxed(c); __iormb(__v); __v; }) > ^ > ./arch/arm64/include/asm/io.h:118:24: note: expanded from macro '__iormb' > asm volatile("eor %0, %1, %1\n" \ > ^ > > > > > Yeah, the message above looks bogus to me. I can see %1 being 32-bit for > > read[bwl], so maybe clang is just getting the diagnostic wrong. If so, > > I wonder if the following fixes the problem: > > > > This doesn't appear to work, I get this error: > > In file included from arch/arm64/kernel/asm-offsets.c:24: > In file included from ./include/linux/dma-mapping.h:11: > In file included from ./include/linux/scatterlist.h:9: > In file included from ./arch/arm64/include/asm/io.h:209: > ./include/asm-generic/io.h:695:9: error: expected expression > return readb(addr); > ^ > ./arch/arm64/include/asm/io.h:147:50: note: expanded from macro 'readb' > #define readb(c) ({ u8 __v = readb_relaxed(c); __iormb(__v); __v; }) > ^ > ./arch/arm64/include/asm/io.h:120:28: note: expanded from macro '__iormb' > : "=r" (tmp) : "r" (unsigned long)(v) : "memory"); \ > ^ Can you try throwing another set of brackets around it, please? ((unsigned long)(v)) Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: io: specify asm operand width for __iormb() 2018-11-29 16:13 ` Will Deacon @ 2018-11-29 16:17 ` Nathan Chancellor 2018-11-29 16:37 ` Will Deacon 0 siblings, 1 reply; 8+ messages in thread From: Nathan Chancellor @ 2018-11-29 16:17 UTC (permalink / raw) To: Will Deacon Cc: Jens Axboe, Julien Thierry, Catalin Marinas, linux-kernel@vger.kernel.org, Nick Desaulniers, linux-arm-kernel@lists.infradead.org On Thu, Nov 29, 2018 at 04:13:37PM +0000, Will Deacon wrote: > On Thu, Nov 29, 2018 at 09:10:39AM -0700, Nathan Chancellor wrote: > > On Thu, Nov 29, 2018 at 10:49:03AM +0000, Will Deacon wrote: > > > On Thu, Nov 29, 2018 at 09:03:54AM +0000, Julien Thierry wrote: > > > > On 29/11/18 04:19, Nick Desaulniers wrote: > > > > > Fixes the warning produced from Clang: > > > > > ./include/asm-generic/io.h:711:9: warning: value size does not match > > > > > register size specified by the constraint and modifier > > > > > [-Wasm-operand-widths] > > > > > return readl(addr); > > > > > ^ > > > > > ./arch/arm64/include/asm/io.h:149:58: note: expanded from macro 'readl' > > > > > ^ > > > > > ./include/asm-generic/io.h:711:9: note: use constraint modifier "w" > > > > > ./arch/arm64/include/asm/io.h:149:50: note: expanded from macro 'readl' > > > > > ^ > > > > > ./arch/arm64/include/asm/io.h:118:25: note: expanded from macro '__iormb' > > > > > asm volatile("eor %w0, %1, %1\n" \ > > > > > ^ > > > > > > > > Why does the "eor %0, %1, %1" become "eor %w0, %1, %1" ? > > > > The variable passed to the inline assembly for %0 is unsigned long, so > > > > always 64-bits wide on arm64. Why is clang trying to use a 32-bit > > > > register for it? > > > > Sorry, this was my fault, I accidentally added a w during testing to see > > what constraints were valid (given that my assembly knowledge is nearly > > non-existence so forgive the non-sensical experimentation) and I used > > that message rather than the original one. This is the unadulterated one. > > Aha, that explains it. Thanks for clearing that up. > > > In file included from arch/arm64/kernel/asm-offsets.c:24: > > In file included from ./include/linux/dma-mapping.h:11: > > In file included from ./include/linux/scatterlist.h:9: > > In file included from ./arch/arm64/include/asm/io.h:209: > > ./include/asm-generic/io.h:695:9: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths] > > return readb(addr); > > ^ > > ./arch/arm64/include/asm/io.h:147:58: note: expanded from macro 'readb' > > #define readb(c) ({ u8 __v = readb_relaxed(c); __iormb(__v); __v; }) > > ^ > > ./include/asm-generic/io.h:695:9: note: use constraint modifier "w" > > ./arch/arm64/include/asm/io.h:147:50: note: expanded from macro 'readb' > > #define readb(c) ({ u8 __v = readb_relaxed(c); __iormb(__v); __v; }) > > ^ > > ./arch/arm64/include/asm/io.h:118:24: note: expanded from macro '__iormb' > > asm volatile("eor %0, %1, %1\n" \ > > ^ > > > > > > > > Yeah, the message above looks bogus to me. I can see %1 being 32-bit for > > > read[bwl], so maybe clang is just getting the diagnostic wrong. If so, > > > I wonder if the following fixes the problem: > > > > > > > This doesn't appear to work, I get this error: > > > > In file included from arch/arm64/kernel/asm-offsets.c:24: > > In file included from ./include/linux/dma-mapping.h:11: > > In file included from ./include/linux/scatterlist.h:9: > > In file included from ./arch/arm64/include/asm/io.h:209: > > ./include/asm-generic/io.h:695:9: error: expected expression > > return readb(addr); > > ^ > > ./arch/arm64/include/asm/io.h:147:50: note: expanded from macro 'readb' > > #define readb(c) ({ u8 __v = readb_relaxed(c); __iormb(__v); __v; }) > > ^ > > ./arch/arm64/include/asm/io.h:120:28: note: expanded from macro '__iormb' > > : "=r" (tmp) : "r" (unsigned long)(v) : "memory"); \ > > ^ > > Can you try throwing another set of brackets around it, please? > > ((unsigned long)(v)) > > Will Thanks, that fixes the warning as well. Nathan _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: io: specify asm operand width for __iormb() 2018-11-29 16:17 ` Nathan Chancellor @ 2018-11-29 16:37 ` Will Deacon 0 siblings, 0 replies; 8+ messages in thread From: Will Deacon @ 2018-11-29 16:37 UTC (permalink / raw) To: Nathan Chancellor Cc: Jens Axboe, Julien Thierry, Catalin Marinas, linux-kernel@vger.kernel.org, Nick Desaulniers, linux-arm-kernel@lists.infradead.org On Thu, Nov 29, 2018 at 09:17:38AM -0700, Nathan Chancellor wrote: > On Thu, Nov 29, 2018 at 04:13:37PM +0000, Will Deacon wrote: > > On Thu, Nov 29, 2018 at 09:10:39AM -0700, Nathan Chancellor wrote: > > > This doesn't appear to work, I get this error: > > > > > > In file included from arch/arm64/kernel/asm-offsets.c:24: > > > In file included from ./include/linux/dma-mapping.h:11: > > > In file included from ./include/linux/scatterlist.h:9: > > > In file included from ./arch/arm64/include/asm/io.h:209: > > > ./include/asm-generic/io.h:695:9: error: expected expression > > > return readb(addr); > > > ^ > > > ./arch/arm64/include/asm/io.h:147:50: note: expanded from macro 'readb' > > > #define readb(c) ({ u8 __v = readb_relaxed(c); __iormb(__v); __v; }) > > > ^ > > > ./arch/arm64/include/asm/io.h:120:28: note: expanded from macro '__iormb' > > > : "=r" (tmp) : "r" (unsigned long)(v) : "memory"); \ > > > ^ > > > > Can you try throwing another set of brackets around it, please? > > > > ((unsigned long)(v)) > > > Thanks, that fixes the warning as well. Great, I'll push this out tomorrow with your reported-by on it. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] arm64: io: specify asm operand width for __iormb() 2018-11-29 16:10 ` Nathan Chancellor 2018-11-29 16:13 ` Will Deacon @ 2018-11-29 16:14 ` Russell King - ARM Linux 1 sibling, 0 replies; 8+ messages in thread From: Russell King - ARM Linux @ 2018-11-29 16:14 UTC (permalink / raw) To: Nathan Chancellor Cc: Jens Axboe, Julien Thierry, Catalin Marinas, Will Deacon, Nick Desaulniers, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org On Thu, Nov 29, 2018 at 09:10:39AM -0700, Nathan Chancellor wrote: > On Thu, Nov 29, 2018 at 10:49:03AM +0000, Will Deacon wrote: > > On Thu, Nov 29, 2018 at 09:03:54AM +0000, Julien Thierry wrote: > > > > > > > > > On 29/11/18 04:19, Nick Desaulniers wrote: > > > > Fixes the warning produced from Clang: > > > > ./include/asm-generic/io.h:711:9: warning: value size does not match > > > > register size specified by the constraint and modifier > > > > [-Wasm-operand-widths] > > > > return readl(addr); > > > > ^ > > > > ./arch/arm64/include/asm/io.h:149:58: note: expanded from macro 'readl' > > > > ^ > > > > ./include/asm-generic/io.h:711:9: note: use constraint modifier "w" > > > > ./arch/arm64/include/asm/io.h:149:50: note: expanded from macro 'readl' > > > > ^ > > > > ./arch/arm64/include/asm/io.h:118:25: note: expanded from macro '__iormb' > > > > asm volatile("eor %w0, %1, %1\n" \ > > > > ^ > > > > > > Why does the "eor %0, %1, %1" become "eor %w0, %1, %1" ? > > > The variable passed to the inline assembly for %0 is unsigned long, so > > > always 64-bits wide on arm64. Why is clang trying to use a 32-bit > > > register for it? > > Sorry, this was my fault, I accidentally added a w during testing to see > what constraints were valid (given that my assembly knowledge is nearly > non-existence so forgive the non-sensical experimentation) and I used > that message rather than the original one. This is the unadulterated one. > > In file included from arch/arm64/kernel/asm-offsets.c:24: > In file included from ./include/linux/dma-mapping.h:11: > In file included from ./include/linux/scatterlist.h:9: > In file included from ./arch/arm64/include/asm/io.h:209: > ./include/asm-generic/io.h:695:9: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths] > return readb(addr); > ^ > ./arch/arm64/include/asm/io.h:147:58: note: expanded from macro 'readb' > #define readb(c) ({ u8 __v = readb_relaxed(c); __iormb(__v); __v; }) > ^ > ./include/asm-generic/io.h:695:9: note: use constraint modifier "w" > ./arch/arm64/include/asm/io.h:147:50: note: expanded from macro 'readb' > #define readb(c) ({ u8 __v = readb_relaxed(c); __iormb(__v); __v; }) > ^ > ./arch/arm64/include/asm/io.h:118:24: note: expanded from macro '__iormb' > asm volatile("eor %0, %1, %1\n" \ > ^ > > > > > Yeah, the message above looks bogus to me. I can see %1 being 32-bit for > > read[bwl], so maybe clang is just getting the diagnostic wrong. If so, > > I wonder if the following fixes the problem: > > > > This doesn't appear to work, I get this error: > > In file included from arch/arm64/kernel/asm-offsets.c:24: > In file included from ./include/linux/dma-mapping.h:11: > In file included from ./include/linux/scatterlist.h:9: > In file included from ./arch/arm64/include/asm/io.h:209: > ./include/asm-generic/io.h:695:9: error: expected expression > return readb(addr); > ^ > ./arch/arm64/include/asm/io.h:147:50: note: expanded from macro 'readb' > #define readb(c) ({ u8 __v = readb_relaxed(c); __iormb(__v); __v; }) > ^ > ./arch/arm64/include/asm/io.h:120:28: note: expanded from macro '__iormb' > : "=r" (tmp) : "r" (unsigned long)(v) : "memory"); \ > ^ > > > > > diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h > > index d42d00d8d5b6..13befec8b64e 100644 > > --- a/arch/arm64/include/asm/io.h > > +++ b/arch/arm64/include/asm/io.h > > @@ -117,7 +117,7 @@ static inline u64 __raw_readq(const volatile void __iomem *addr) > > */ \ > > asm volatile("eor %0, %1, %1\n" \ > > "cbnz %0, ." \ > > - : "=r" (tmp) : "r" (v) : "memory"); \ > > + : "=r" (tmp) : "r" (unsigned long)(v) : "memory"); \ The parens around the passed value are part of the asm() syntax, which is: "contraint" (expression) so should be: + : "=r" (tmp) : "r" ((unsigned long)(v)) : "memory"); \ -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-11-29 16:37 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-11-29 4:19 [PATCH] arm64: io: specify asm operand width for __iormb() Nick Desaulniers 2018-11-29 9:03 ` Julien Thierry 2018-11-29 10:49 ` Will Deacon 2018-11-29 16:10 ` Nathan Chancellor 2018-11-29 16:13 ` Will Deacon 2018-11-29 16:17 ` Nathan Chancellor 2018-11-29 16:37 ` Will Deacon 2018-11-29 16:14 ` Russell King - ARM Linux
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).