[geert-m68k:master 2/2] lib/muldi3.c:53:28: sparse: sparse: asm output is not an lvalue

[geert-m68k:master 2/2] lib/muldi3.c:53:28: sparse: sparse: asm output is not an lvalue

[kernel test robot]

Hi Greg,

First bad commit (maybe != root cause):

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k.git master
head:   17810b1de0e9e5c1000cf8f5b4787ad930b0353e
commit: e96424b86d5098f44279399b85551e0f84a1c9e9 [2/2] m68k: Use kernel's generic muldi3 libgcc function
config: m68k-randconfig-r122-20250101 (https://download.01.org/0day-ci/archive/20250101/202501011517.5edVb87Z-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 14.2.0
reproduce: (https://download.01.org/0day-ci/archive/20250101/202501011517.5edVb87Z-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501011517.5edVb87Z-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> lib/muldi3.c:53:28: sparse: sparse: asm output is not an lvalue
>> lib/muldi3.c:53:28: sparse: sparse: asm output is not an lvalue
>> lib/muldi3.c:53:28: sparse: sparse: not addressable
>> lib/muldi3.c:53:28: sparse: sparse: generating address of non-lvalue (11)
>> lib/muldi3.c:53:28: sparse: sparse: generating address of non-lvalue (11)

vim +53 lib/muldi3.c

b35cd9884fa5d8 Palmer Dabbelt 2017-05-23  48  
b35cd9884fa5d8 Palmer Dabbelt 2017-05-23  49  long long notrace __muldi3(long long u, long long v)
b35cd9884fa5d8 Palmer Dabbelt 2017-05-23  50  {
b35cd9884fa5d8 Palmer Dabbelt 2017-05-23  51  	const DWunion uu = {.ll = u};
b35cd9884fa5d8 Palmer Dabbelt 2017-05-23  52  	const DWunion vv = {.ll = v};
b35cd9884fa5d8 Palmer Dabbelt 2017-05-23 @53  	DWunion w = {.ll = __umulsidi3(uu.s.low, vv.s.low)};

:::::: The code at line 53 was first introduced by commit
:::::: b35cd9884fa5d81c9d5e7f57c9d03264ae2bd835 lib: Add shared copies of some GCC library routines

:::::: TO: Palmer Dabbelt <palmer@dabbelt.com>
:::::: CC: Palmer Dabbelt <palmer@dabbelt.com>

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[geert-m68k:master 2/2] lib/muldi3.c:53:28: sparse: sparse: asm output is not an lvalue

[kernel test robot]

Hi Greg,

First bad commit (maybe != root cause):

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k.git master
head:   17810b1de0e9e5c1000cf8f5b4787ad930b0353e
commit: e96424b86d5098f44279399b85551e0f84a1c9e9 [2/2] m68k: Use kernel's generic muldi3 libgcc function
config: m68k-randconfig-r122-20250101 (https://download.01.org/0day-ci/archive/20250103/202501030516.uZrwnuQQ-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 14.2.0
reproduce: (https://download.01.org/0day-ci/archive/20250103/202501030516.uZrwnuQQ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501030516.uZrwnuQQ-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> lib/muldi3.c:53:28: sparse: sparse: asm output is not an lvalue
>> lib/muldi3.c:53:28: sparse: sparse: asm output is not an lvalue
>> lib/muldi3.c:53:28: sparse: sparse: not addressable
>> lib/muldi3.c:53:28: sparse: sparse: generating address of non-lvalue (11)
>> lib/muldi3.c:53:28: sparse: sparse: generating address of non-lvalue (11)

vim +53 lib/muldi3.c

b35cd9884fa5d81 Palmer Dabbelt 2017-05-23  48  
b35cd9884fa5d81 Palmer Dabbelt 2017-05-23  49  long long notrace __muldi3(long long u, long long v)
b35cd9884fa5d81 Palmer Dabbelt 2017-05-23  50  {
b35cd9884fa5d81 Palmer Dabbelt 2017-05-23  51  	const DWunion uu = {.ll = u};
b35cd9884fa5d81 Palmer Dabbelt 2017-05-23  52  	const DWunion vv = {.ll = v};
b35cd9884fa5d81 Palmer Dabbelt 2017-05-23 @53  	DWunion w = {.ll = __umulsidi3(uu.s.low, vv.s.low)};

:::::: The code at line 53 was first introduced by commit
:::::: b35cd9884fa5d81c9d5e7f57c9d03264ae2bd835 lib: Add shared copies of some GCC library routines

:::::: TO: Palmer Dabbelt <palmer@dabbelt.com>
:::::: CC: Palmer Dabbelt <palmer@dabbelt.com>

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [geert-m68k:master 2/2] lib/muldi3.c:53:28: sparse: sparse: asm output is not an lvalue

[Geert Uytterhoeven]

Hi Kernel test robot,

CC linux-sparse

On Thu, Jan 2, 2025 at 10:40 PM kernel test robot <lkp@intel.com> wrote:
> First bad commit (maybe != root cause):
>
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k.git master
> head:   17810b1de0e9e5c1000cf8f5b4787ad930b0353e
> commit: e96424b86d5098f44279399b85551e0f84a1c9e9 [2/2] m68k: Use kernel's generic muldi3 libgcc function
> config: m68k-randconfig-r122-20250101 (https://download.01.org/0day-ci/archive/20250103/202501030516.uZrwnuQQ-lkp@intel.com/config)
> compiler: m68k-linux-gcc (GCC) 14.2.0
> reproduce: (https://download.01.org/0day-ci/archive/20250103/202501030516.uZrwnuQQ-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202501030516.uZrwnuQQ-lkp@intel.com/
>
> sparse warnings: (new ones prefixed by >>)
> >> lib/muldi3.c:53:28: sparse: sparse: asm output is not an lvalue
> >> lib/muldi3.c:53:28: sparse: sparse: asm output is not an lvalue
> >> lib/muldi3.c:53:28: sparse: sparse: not addressable
> >> lib/muldi3.c:53:28: sparse: sparse: generating address of non-lvalue (11)
> >> lib/muldi3.c:53:28: sparse: sparse: generating address of non-lvalue (11)
>
> vim +53 lib/muldi3.c
>
> b35cd9884fa5d81 Palmer Dabbelt 2017-05-23  48
> b35cd9884fa5d81 Palmer Dabbelt 2017-05-23  49  long long notrace __muldi3(long long u, long long v)
> b35cd9884fa5d81 Palmer Dabbelt 2017-05-23  50  {
> b35cd9884fa5d81 Palmer Dabbelt 2017-05-23  51   const DWunion uu = {.ll = u};
> b35cd9884fa5d81 Palmer Dabbelt 2017-05-23  52   const DWunion vv = {.ll = v};
> b35cd9884fa5d81 Palmer Dabbelt 2017-05-23 @53   DWunion w = {.ll = __umulsidi3(uu.s.low, vv.s.low)};
>
> :::::: The code at line 53 was first introduced by commit
> :::::: b35cd9884fa5d81c9d5e7f57c9d03264ae2bd835 lib: Add shared copies of some GCC library routines

Thanks for the report!

Comparing m68k to the other two architectures that use
GENERIC_LIB_MULDI3 (csky and xtensa), and to microblaze (which has
its own arch/microblaze/lib/muldi3.c), that don't trigger this error,
the difference is that m68k uses inline assembler in its umul_ppmm()
implementation. I don't see anything that's wrong with it, though.

Perhaps this is an issue with sparse?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [geert-m68k:master 2/2] lib/muldi3.c:53:28: sparse: sparse: asm output is not an lvalue

[Linus Torvalds]

On Fri, 3 Jan 2025 at 01:22, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Perhaps this is an issue with sparse?

No, this is just m68k doing disgusting things that happen to work with
gcc, because gcc considers casts to be lvalues (which in turn is comes
from some horrid C++ thing, since in C++ casting is a whole magical
extra complexity).

IOW, this m68k pattern is horrendous, and sparse quite reasonably
complains about it:

  #define umul_ppmm(w1, w0, u, v) \
    __asm__ ("mulu%.l %3,%1:%0"                                           \
             : "=d" ((USItype)(w0)),                                      \
               "=d" ((USItype)(w1))                                       \
             : "%0" ((USItype)(u)),                                       \
               "dmi" ((USItype)(v)))

notice how it has two register outputs (the "=d"), and the destination
of said output is not a proper lvalue, but a cast expression.

I think you could just remove the cast. Afaik the w0/w1 arguments come from

#define __umulsidi3(u, v) \
  ({DIunion __w;                                                        \
    umul_ppmm (__w.s.high, __w.s.low, u, v);                            \
    __w.ll; })

and __w.s.high and __w.s.low are from struct DIstruct, which uses
"SItype". So all the cast does is to change the signedness of the
variable, but that has no *meaning* when you assign to it and the
sizes match.

Alternatively, you could just use the right types explicitly and write
the umul_ppmm() macro something like

#define umul_ppmm(w1, w0, u, v) do {            \
        USItype __w0, __w1;                     \
        __asm__ ("mulu%.l %3,%1:%0"             \
                : "=d" (__w0)                   \
                  "=d" (__w1)                   \
                : "%0" ((USItype)(u)),          \
                  "dmi" ((USItype)(v)));        \
        w0 = __w0; w1 = __w1; } while (0)

NOTE! UNTESTED! Treat the above as a "something like this, perhaps".

              Linus

Re: [geert-m68k:master 2/2] lib/muldi3.c:53:28: sparse: sparse: asm output is not an lvalue

[Geert Uytterhoeven]

Hi Linus,

On Sun, Jan 5, 2025 at 10:51 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Fri, 3 Jan 2025 at 01:22, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > Perhaps this is an issue with sparse?
>
> No, this is just m68k doing disgusting things that happen to work with
> gcc, because gcc considers casts to be lvalues (which in turn is comes
> from some horrid C++ thing, since in C++ casting is a whole magical
> extra complexity).
>
> IOW, this m68k pattern is horrendous, and sparse quite reasonably
> complains about it:
>
>   #define umul_ppmm(w1, w0, u, v) \
>     __asm__ ("mulu%.l %3,%1:%0"                                           \
>              : "=d" ((USItype)(w0)),                                      \
>                "=d" ((USItype)(w1))                                       \
>              : "%0" ((USItype)(u)),                                       \
>                "dmi" ((USItype)(v)))
>
> notice how it has two register outputs (the "=d"), and the destination
> of said output is not a proper lvalue, but a cast expression.
>
> I think you could just remove the cast. Afaik the w0/w1 arguments come from
>
> #define __umulsidi3(u, v) \
>   ({DIunion __w;                                                        \
>     umul_ppmm (__w.s.high, __w.s.low, u, v);                            \
>     __w.ll; })
>
> and __w.s.high and __w.s.low are from struct DIstruct, which uses
> "SItype". So all the cast does is to change the signedness of the
> variable, but that has no *meaning* when you assign to it and the
> sizes match.
>
> Alternatively, you could just use the right types explicitly and write
> the umul_ppmm() macro something like
>
> #define umul_ppmm(w1, w0, u, v) do {            \
>         USItype __w0, __w1;                     \
>         __asm__ ("mulu%.l %3,%1:%0"             \
>                 : "=d" (__w0)                   \
>                   "=d" (__w1)                   \
>                 : "%0" ((USItype)(u)),          \
>                   "dmi" ((USItype)(v)));        \
>         w0 = __w0; w1 = __w1; } while (0)
>
> NOTE! UNTESTED! Treat the above as a "something like this, perhaps".

Thank you, using intermediate variables instead of casts for the output
operands gets rid of the lvalue-related sparse warnings.
That leaves us with the "not addressable" sparse error.  Apparently
that goes away by dropping the cast on the last input operand,
so I will introduce intermediate variables for all input operands, too.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [geert-m68k:master 2/2] lib/muldi3.c:53:28: sparse: sparse: asm output is not an lvalue

[Linus Torvalds]

On Wed, 8 Jan 2025 at 06:19, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> That leaves us with the "not addressable" sparse error.

Hmm. That one is actually a sparse mis-feature.

Sparse sees that

          "dmi" ((USItype)(v))

input, and the 'm' makes it go "it's a memory location" and that makes
sparse go "it must be addressable".

But for asm inputs, it just means that the compiler should *put* the
thing in memory to be an input.

And this is hidden on x86, because sparse recognizes "r" for register,
and says "if it can be either a register or memory, I don't require
memory". So there's various x86 inline asm that does something like

        "rm" (0)

and sparse won't complain about the zero not being addressable.

But in your case, just making the inputs lvalues will fix the sparse
problem, so I guess that's the right thing to do. I sadly don't see
sparse being fixed because we don't have a maintainer..

                 Linus