* [PATCH] riscv: pass machine size to sparse @ 2018-05-28 16:35 Luc Van Oostenryck 2018-05-29 6:11 ` Christoph Hellwig 0 siblings, 1 reply; 6+ messages in thread From: Luc Van Oostenryck @ 2018-05-28 16:35 UTC (permalink / raw) To: linux-riscv By default, sparse assumes a 64bit machine when compiled on x86-64 and 32bit when compiled on anything else. This can of course create all sort of problems when this doesn't correspond to the target's machine size, like issuing false warnings like: 'shift too big (32) for type unsigned long' or is 64bit while sparse was compiled on a 32bit machine, or worse, to not emit legitimate warnings. Fix this by passing the appropriate -m32/-m64 flag to sparse. Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> --- arch/riscv/Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile index 76e958a54..cb2502e4c 100644 --- a/arch/riscv/Makefile +++ b/arch/riscv/Makefile @@ -71,6 +71,8 @@ KBUILD_CFLAGS_MODULE += $(call cc-option,-mno-relax) # architectures. It's faster to have GCC emit only aligned accesses. KBUILD_CFLAGS += $(call cc-option,-mstrict-align) +CHECKFLAGS += -m$(BITS) + head-y := arch/riscv/kernel/head.o core-y += arch/riscv/kernel/ arch/riscv/mm/ -- 2.17.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] riscv: pass machine size to sparse 2018-05-28 16:35 [PATCH] riscv: pass machine size to sparse Luc Van Oostenryck @ 2018-05-29 6:11 ` Christoph Hellwig 2018-05-29 6:14 ` Masahiro Yamada 0 siblings, 1 reply; 6+ messages in thread From: Christoph Hellwig @ 2018-05-29 6:11 UTC (permalink / raw) To: linux-riscv On Mon, May 28, 2018 at 06:35:05PM +0200, Luc Van Oostenryck wrote: > By default, sparse assumes a 64bit machine when compiled on x86-64 > and 32bit when compiled on anything else. > > This can of course create all sort of problems when this doesn't > correspond to the target's machine size, like issuing false > warnings like: 'shift too big (32) for type unsigned long' or > is 64bit while sparse was compiled on a 32bit machine, or worse, > to not emit legitimate warnings. > > Fix this by passing the appropriate -m32/-m64 flag to sparse. Can we please move this to the common Kbuild code using the CONFIG_64BIT syombol? This really should not need boiler plate in every architecture. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] riscv: pass machine size to sparse 2018-05-29 6:11 ` Christoph Hellwig @ 2018-05-29 6:14 ` Masahiro Yamada 2018-05-31 12:09 ` Palmer Dabbelt 0 siblings, 1 reply; 6+ messages in thread From: Masahiro Yamada @ 2018-05-29 6:14 UTC (permalink / raw) To: linux-riscv 2018-05-29 15:11 GMT+09:00 Christoph Hellwig <hch@infradead.org>: > On Mon, May 28, 2018 at 06:35:05PM +0200, Luc Van Oostenryck wrote: >> By default, sparse assumes a 64bit machine when compiled on x86-64 >> and 32bit when compiled on anything else. >> >> This can of course create all sort of problems when this doesn't >> correspond to the target's machine size, like issuing false >> warnings like: 'shift too big (32) for type unsigned long' or >> is 64bit while sparse was compiled on a 32bit machine, or worse, >> to not emit legitimate warnings. >> >> Fix this by passing the appropriate -m32/-m64 flag to sparse. > > Can we please move this to the common Kbuild code using the > CONFIG_64BIT syombol? This really should not need boiler plate in > every architecture. I agree. Luc did so for -mbig/little-endian: https://patchwork.kernel.org/patch/10433957/ We should do likewise for -m32/64. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] riscv: pass machine size to sparse 2018-05-29 6:14 ` Masahiro Yamada @ 2018-05-31 12:09 ` Palmer Dabbelt 2018-05-31 15:35 ` Luc Van Oostenryck 0 siblings, 1 reply; 6+ messages in thread From: Palmer Dabbelt @ 2018-05-31 12:09 UTC (permalink / raw) To: linux-riscv On Mon, 28 May 2018 23:14:20 PDT (-0700), yamada.masahiro at socionext.com wrote: > 2018-05-29 15:11 GMT+09:00 Christoph Hellwig <hch@infradead.org>: >> On Mon, May 28, 2018 at 06:35:05PM +0200, Luc Van Oostenryck wrote: >>> By default, sparse assumes a 64bit machine when compiled on x86-64 >>> and 32bit when compiled on anything else. >>> >>> This can of course create all sort of problems when this doesn't >>> correspond to the target's machine size, like issuing false >>> warnings like: 'shift too big (32) for type unsigned long' or >>> is 64bit while sparse was compiled on a 32bit machine, or worse, >>> to not emit legitimate warnings. >>> >>> Fix this by passing the appropriate -m32/-m64 flag to sparse. >> >> Can we please move this to the common Kbuild code using the >> CONFIG_64BIT syombol? This really should not need boiler plate in >> every architecture. > > > I agree. > > Luc did so for -mbig/little-endian: > https://patchwork.kernel.org/patch/10433957/ > > We should do likewise for -m32/64. Sorry for being a bit slow here, but I like the idea of making the 32/64-bit issue generic as it seems like it'll be necessary for every port. Looking through the patch for big/little-endian I did notice: * RISC-V compilers set "__riscv_xlen" to the length of an X (integer) register in bits. * RISC-V compilers define "__riscv", and it doesn't appear we inform sparse about that. These two might not be that interesting, but we do already have some cases where we're checking for __riscv_xlen in Linux. I've yet to successfully use sparse, but adding at least CHECKFLAGS += -D__riscv seems reasonable, and possibly also some sort of ifeq ($(CONFIG_ARCH_RV64I),y) CHECKFLAGS += -D__riscv_xlen=64 else CHECKFLAGS += -D__riscv_xlen=32 fi might be necessary. We strive to follow the generic rules for ABI-related stuff like __LP64__ but I don't think there's any generic mapping for XLEN. Similarly there's "__riscv_flen" and "__riscv_float_abi_*", but those are less likely to be used by the kernel so they're probably not worth worrying about for now. There's also a bunch of other RISC-V macros, the only one of which we're currently using is "__riscv_muldiv" (and that's in a manner that's unlikely to trigger any sort of static analysis). Between a lack of Kconfig options and a glibc port we're essentially mandating IMA right now, so these probably don't matter. Thanks! ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] riscv: pass machine size to sparse 2018-05-31 12:09 ` Palmer Dabbelt @ 2018-05-31 15:35 ` Luc Van Oostenryck 2018-06-05 1:18 ` Palmer Dabbelt 0 siblings, 1 reply; 6+ messages in thread From: Luc Van Oostenryck @ 2018-05-31 15:35 UTC (permalink / raw) To: linux-riscv On Thu, May 31, 2018 at 05:09:21AM -0700, Palmer Dabbelt wrote: > On Mon, 28 May 2018 23:14:20 PDT (-0700), yamada.masahiro at socionext.com wrote: > > 2018-05-29 15:11 GMT+09:00 Christoph Hellwig <hch@infradead.org>: > > > On Mon, May 28, 2018 at 06:35:05PM +0200, Luc Van Oostenryck wrote: > > > > By default, sparse assumes a 64bit machine when compiled on x86-64 > > > > and 32bit when compiled on anything else. > > > > > > > > This can of course create all sort of problems when this doesn't > > > > correspond to the target's machine size, like issuing false > > > > warnings like: 'shift too big (32) for type unsigned long' or > > > > is 64bit while sparse was compiled on a 32bit machine, or worse, > > > > to not emit legitimate warnings. > > > > > > > > Fix this by passing the appropriate -m32/-m64 flag to sparse. > > > > > > Can we please move this to the common Kbuild code using the > > > CONFIG_64BIT syombol? This really should not need boiler plate in > > > every architecture. > > > > > > I agree. > > > > Luc did so for -mbig/little-endian: > > https://patchwork.kernel.org/patch/10433957/ > > > > We should do likewise for -m32/64. > > Sorry for being a bit slow here, but I like the idea of making the > 32/64-bit issue generic as it seems like it'll be necessary for > every port. Sure, Christophe asked it too. I've sent a new version doing it once and for all for all archs but I forgot to CC you: https://lkml.org/lkml/2018/5/30/948 > Looking through the patch for big/little-endian I did > notice: > > * RISC-V compilers set "__riscv_xlen" to the length of an X > (integer) register in bits. > * RISC-V compilers define "__riscv", and it doesn't appear we inform > sparse about that. > > These two might not be that interesting, but we do already have some > cases where we're checking for __riscv_xlen in Linux. I've yet to > successfully use sparse, but adding at least > > CHECKFLAGS += -D__riscv > > seems reasonable, Sure (but I don't see a dependency in the kernel (yet)). > and possibly also some sort of > > ifeq ($(CONFIG_ARCH_RV64I),y) > CHECKFLAGS += -D__riscv_xlen=64 > else > CHECKFLAGS += -D__riscv_xlen=32 > fi > > might be necessary. Yes, this one is really needed. I'll send a patch for this one and __riscv. > We strive to follow the generic rules for > ABI-related stuff like __LP64__ but I don't think there's any > generic mapping for XLEN. Similarly there's "__riscv_flen" and > "__riscv_float_abi_*", but those are less likely to be used by the > kernel so they're probably not worth worrying about for now. Yes, I agree. Note that sparse will define __LP64__ (and _LP64) when in -m64 mode. > There's also a bunch of other RISC-V macros, the only one of which > we're currently using is "__riscv_muldiv" (and that's in a manner > that's unlikely to trigger any sort of static analysis). Between a > lack of Kconfig options and a glibc port we're essentially mandating > IMA right now, so these probably don't matter. Yes, I just saw. I think also it's better to leave it so for now. And if it becomes more used, then better to infer it from the compiler than harcoding it. Regards, -- Luc ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] riscv: pass machine size to sparse 2018-05-31 15:35 ` Luc Van Oostenryck @ 2018-06-05 1:18 ` Palmer Dabbelt 0 siblings, 0 replies; 6+ messages in thread From: Palmer Dabbelt @ 2018-06-05 1:18 UTC (permalink / raw) To: linux-riscv On Thu, 31 May 2018 08:35:54 PDT (-0700), luc.vanoostenryck at gmail.com wrote: > On Thu, May 31, 2018 at 05:09:21AM -0700, Palmer Dabbelt wrote: >> On Mon, 28 May 2018 23:14:20 PDT (-0700), yamada.masahiro at socionext.com wrote: >> > 2018-05-29 15:11 GMT+09:00 Christoph Hellwig <hch@infradead.org>: >> > > On Mon, May 28, 2018 at 06:35:05PM +0200, Luc Van Oostenryck wrote: >> > > > By default, sparse assumes a 64bit machine when compiled on x86-64 >> > > > and 32bit when compiled on anything else. >> > > > >> > > > This can of course create all sort of problems when this doesn't >> > > > correspond to the target's machine size, like issuing false >> > > > warnings like: 'shift too big (32) for type unsigned long' or >> > > > is 64bit while sparse was compiled on a 32bit machine, or worse, >> > > > to not emit legitimate warnings. >> > > > >> > > > Fix this by passing the appropriate -m32/-m64 flag to sparse. >> > > >> > > Can we please move this to the common Kbuild code using the >> > > CONFIG_64BIT syombol? This really should not need boiler plate in >> > > every architecture. >> > >> > >> > I agree. >> > >> > Luc did so for -mbig/little-endian: >> > https://patchwork.kernel.org/patch/10433957/ >> > >> > We should do likewise for -m32/64. >> >> Sorry for being a bit slow here, but I like the idea of making the >> 32/64-bit issue generic as it seems like it'll be necessary for >> every port. > > Sure, Christophe asked it too. > I've sent a new version doing it once and for all for all archs > but I forgot to CC you: > https://lkml.org/lkml/2018/5/30/948 > >> Looking through the patch for big/little-endian I did >> notice: >> >> * RISC-V compilers set "__riscv_xlen" to the length of an X >> (integer) register in bits. >> * RISC-V compilers define "__riscv", and it doesn't appear we inform >> sparse about that. >> >> These two might not be that interesting, but we do already have some >> cases where we're checking for __riscv_xlen in Linux. I've yet to >> successfully use sparse, but adding at least >> >> CHECKFLAGS += -D__riscv >> >> seems reasonable, > > Sure (but I don't see a dependency in the kernel (yet)). > >> and possibly also some sort of >> >> ifeq ($(CONFIG_ARCH_RV64I),y) >> CHECKFLAGS += -D__riscv_xlen=64 >> else >> CHECKFLAGS += -D__riscv_xlen=32 >> fi >> >> might be necessary. > > Yes, this one is really needed. > I'll send a patch for this one and __riscv. > >> We strive to follow the generic rules for >> ABI-related stuff like __LP64__ but I don't think there's any >> generic mapping for XLEN. Similarly there's "__riscv_flen" and >> "__riscv_float_abi_*", but those are less likely to be used by the >> kernel so they're probably not worth worrying about for now. > > Yes, I agree. > Note that sparse will define __LP64__ (and _LP64) when in -m64 mode. > >> There's also a bunch of other RISC-V macros, the only one of which >> we're currently using is "__riscv_muldiv" (and that's in a manner >> that's unlikely to trigger any sort of static analysis). Between a >> lack of Kconfig options and a glibc port we're essentially mandating >> IMA right now, so these probably don't matter. > > Yes, I just saw. I think also it's better to leave it so for now. > And if it becomes more used, then better to infer it from the compiler > than harcoding it. Makes sense. Thanks for the work! ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-06-05 1:18 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-05-28 16:35 [PATCH] riscv: pass machine size to sparse Luc Van Oostenryck 2018-05-29 6:11 ` Christoph Hellwig 2018-05-29 6:14 ` Masahiro Yamada 2018-05-31 12:09 ` Palmer Dabbelt 2018-05-31 15:35 ` Luc Van Oostenryck 2018-06-05 1:18 ` Palmer Dabbelt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox