Depending on register assignment by the compiler:
{standard input}:3084: Error: operands mismatch -- statement `andl %a1,%d1' ignored
{standard input}:3145: Error: operands mismatch -- statement `orl %a1,%d1' ignored
{standard input}:3195: Error: operands mismatch -- statement `eorl %a1,%d1' ignored
Indeed, the first operand must not be an address register. Fix this by
adjusting the register constraint from "g" (general purpose register) to
"d" (data register).
Fixes: e39d88ea3ce4a471 ("locking/atomic, arch/m68k: Implement atomic_fetch_{add,sub,and,or,xor}()")
Fixes: d839bae4269aea46 ("locking,arch,m68k: Fold atomic_ops")
Reported-by: kernel test robot <[email protected]>
Reported-by: Arnd Bergmann <[email protected]>
Reported-by: Alexander Viro <[email protected]>
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
Personally, I have never seen this failure in an 68020+ build, but I can
reproduce it on Coldfire with the config provided by lkp (with bogus
CONFIG_RMW_INSNS=y).
Technically, the issue was present before, but I doubt adding pre-v3.18
Fixes tags would make any difference for stable...
---
arch/m68k/include/asm/atomic.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/m68k/include/asm/atomic.h b/arch/m68k/include/asm/atomic.h
index 8637bf8a2f652009..2d5a6e556754290b 100644
--- a/arch/m68k/include/asm/atomic.h
+++ b/arch/m68k/include/asm/atomic.h
@@ -48,7 +48,7 @@ static inline int arch_atomic_##op##_return(int i, atomic_t *v) \
" casl %2,%1,%0\n" \
" jne 1b" \
: "+m" (*v), "=&d" (t), "=&d" (tmp) \
- : "g" (i), "2" (arch_atomic_read(v))); \
+ : "d" (i), "2" (arch_atomic_read(v))); \
return t; \
}
@@ -63,7 +63,7 @@ static inline int arch_atomic_fetch_##op(int i, atomic_t *v) \
" casl %2,%1,%0\n" \
" jne 1b" \
: "+m" (*v), "=&d" (t), "=&d" (tmp) \
- : "g" (i), "2" (arch_atomic_read(v))); \
+ : "d" (i), "2" (arch_atomic_read(v))); \
return tmp; \
}
--
2.25.1
On Jul 25 2021, Geert Uytterhoeven wrote:
> Depending on register assignment by the compiler:
>
> {standard input}:3084: Error: operands mismatch -- statement `andl %a1,%d1' ignored
> {standard input}:3145: Error: operands mismatch -- statement `orl %a1,%d1' ignored
> {standard input}:3195: Error: operands mismatch -- statement `eorl %a1,%d1' ignored
>
> Indeed, the first operand must not be an address register. Fix this by
> adjusting the register constraint from "g" (general purpose register) to
> "d" (data register).
You should also allow immediate ("i").
There is the ASM_DI macro for that, but since CONFIG_RMW_INSNS is never
defined for CONFIG_COLDFIRE, it probably doesn't matter.
Andreas.
--
Andreas Schwab, [email protected]
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
On Sun, Jul 25, 2021 at 12:46 PM Geert Uytterhoeven
<[email protected]> wrote:
>
> Depending on register assignment by the compiler:
>
> {standard input}:3084: Error: operands mismatch -- statement `andl %a1,%d1' ignored
> {standard input}:3145: Error: operands mismatch -- statement `orl %a1,%d1' ignored
> {standard input}:3195: Error: operands mismatch -- statement `eorl %a1,%d1' ignored
>
> Indeed, the first operand must not be an address register. Fix this by
> adjusting the register constraint from "g" (general purpose register) to
> "d" (data register).
>
> Fixes: e39d88ea3ce4a471 ("locking/atomic, arch/m68k: Implement atomic_fetch_{add,sub,and,or,xor}()")
> Fixes: d839bae4269aea46 ("locking,arch,m68k: Fold atomic_ops")
> Reported-by: kernel test robot <[email protected]>
> Reported-by: Arnd Bergmann <[email protected]>
> Reported-by: Alexander Viro <[email protected]>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> Personally, I have never seen this failure in an 68020+ build, but I can
> reproduce it on Coldfire with the config provided by lkp (with bogus
> CONFIG_RMW_INSNS=y).
This fixed the problem for me in the 68020 build with all compiler versions.
Tested-by: Arnd Bergmann <[email protected]>
On Sun, 25 Jul 2021, Geert Uytterhoeven wrote:
> Fixes: d839bae4269aea46 ("locking,arch,m68k: Fold atomic_ops")
> ...
> Technically, the issue was present before, but I doubt adding pre-v3.18
> Fixes tags would make any difference for stable...
There is a better way to constrain backporting, that is Cc:
[email protected] # 3.12+
The reason I mention it is that Fixes tags could be seen as a way to
identify commits that introduce bugs, e.g. for the purposes of training
AIs, or attributing blame, or measuring quality etc. I think it would be
unfair to point the finger at Peter's commit.
Hi Andreas,
On Sun, Jul 25, 2021 at 4:24 PM Andreas Schwab <[email protected]> wrote:
> On Jul 25 2021, Geert Uytterhoeven wrote:
> > Depending on register assignment by the compiler:
> >
> > {standard input}:3084: Error: operands mismatch -- statement `andl %a1,%d1' ignored
> > {standard input}:3145: Error: operands mismatch -- statement `orl %a1,%d1' ignored
> > {standard input}:3195: Error: operands mismatch -- statement `eorl %a1,%d1' ignored
> >
> > Indeed, the first operand must not be an address register. Fix this by
> > adjusting the register constraint from "g" (general purpose register) to
> > "d" (data register).
>
> You should also allow immediate ("i").
Good point.
> There is the ASM_DI macro for that, but since CONFIG_RMW_INSNS is never
> defined for CONFIG_COLDFIRE, it probably doesn't matter.
As the second operand is a register, not memory, there is no need to
use ASM_DI, and "di" should be fine, right?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
Hi Finn,
On Mon, Jul 26, 2021 at 1:45 AM Finn Thain <[email protected]> wrote:
> On Sun, 25 Jul 2021, Geert Uytterhoeven wrote:
> > Fixes: d839bae4269aea46 ("locking,arch,m68k: Fold atomic_ops")
> > ...
> > Technically, the issue was present before, but I doubt adding pre-v3.18
> > Fixes tags would make any difference for stable...
>
> There is a better way to constrain backporting, that is Cc:
> [email protected] # 3.12+
I don't want to constrain backporting.
> The reason I mention it is that Fixes tags could be seen as a way to
> identify commits that introduce bugs, e.g. for the purposes of training
OK, had a look through the full log....
There are no other commits introducing the bug (renaming and
merging files without changing functions doesn't count), except for the
initial import into git. So I'll add that one, too.
> AIs, or attributing blame, or measuring quality etc. I think it would be
> unfair to point the finger at Peter's commit.
The first Fixes commit definitely introduced a new buggy user.
The second Fixes commit is debatable, as it copied the bug for a new
function from two functions that were removed in the process.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
On Mon, Jul 26, 2021 at 9:34 AM Geert Uytterhoeven <[email protected]> wrote:
>
> Hi Finn,
>
> On Mon, Jul 26, 2021 at 1:45 AM Finn Thain <[email protected]> wrote:
> > On Sun, 25 Jul 2021, Geert Uytterhoeven wrote:
> > > Fixes: d839bae4269aea46 ("locking,arch,m68k: Fold atomic_ops")
> > > ...
> > > Technically, the issue was present before, but I doubt adding pre-v3.18
> > > Fixes tags would make any difference for stable...
> >
> > There is a better way to constrain backporting, that is Cc:
> > [email protected] # 3.12+
>
> I don't want to constrain backporting.
I would recommend adding the plain
Cc: [email protected]
line to the footer to make it unambiguous that you want it backported then,
plus moving the explanation above the --- line. You can never be too explicit
with those.
Arnd