From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
To: Palmer Dabbelt <palmer@sifive.com>
Cc: yamada.masahiro@socionext.com,
Christoph Hellwig <hch@infradead.org>,
linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
albert@sifive.com, michal.lkml@markovi.net
Subject: Re: [PATCH] riscv: pass machine size to sparse
Date: Thu, 31 May 2018 17:35:54 +0200 [thread overview]
Message-ID: <20180531153553.kex32gupopdq2nqh@ltop.local> (raw)
In-Reply-To: <mhng-a4f5b769-8271-4b99-ae61-bbc49fa2286f@palmer-si-x1c4>
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@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
next prev parent reply other threads:[~2018-05-31 15:35 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2018-06-05 1:18 ` Palmer Dabbelt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180531153553.kex32gupopdq2nqh@ltop.local \
--to=luc.vanoostenryck@gmail.com \
--cc=albert@sifive.com \
--cc=hch@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=michal.lkml@markovi.net \
--cc=palmer@sifive.com \
--cc=yamada.masahiro@socionext.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).