RISCV64 32-bit AMOs incorrectly simulated

Bug #1918026 reported by Ryan Macnak
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
QEMU
Fix Released
Undecided
Unassigned

Bug Description

Version: qemu-riscv64 version 4.2.1 (Debian 1:4.2-3ubuntu6.14)

test:
  amomaxu.w a0, a1, (a0)
  ret

int32_t* value = -7;
EXPECT_EQ(-7, test(&value, -11));
EXPECT_EQ(-7, value); // FAIL, saw -11
EXPECT_EQ(-7, test(&value, -7));
EXPECT_EQ(-7, value); // FAIL, raw -11
EXPECT_EQ(-7, test(&value, -4));
EXPECT_EQ(-4, value);

test:
  amomax.w a0, a1, (a0)
  ret

int32_t* value = -7;
EXPECT_EQ(-7, test(&value, -11));
EXPECT_EQ(-7, value);
EXPECT_EQ(-7, test(&value, -7));
EXPECT_EQ(-7, value);
EXPECT_EQ(-7, test(&value, -4));
EXPECT_EQ(-4, value); // FAIL, saw -7

I suspect that trans_amo<op>_w should be using tcg_gen_atomic_fetch_<op>_i32 instead of tcg_gen_atomic_fetch_<op>_tl.

Revision history for this message
Richard Henderson (rth) wrote :

Not as simple as _tl vs _i32. That suffix should be taking
care of the sign-extension to _tl that happens after the
operation. The size of the operation should be in the MO_TESL,
which specifies target-endian signed "long" (32-bit).

Will investigate further.

Revision history for this message
Richard Henderson (rth) wrote :
  • z.c Edit (784 bytes, text/x-csrc)

Flushing out the report to something that compiles,
the test case works for me using qemu 5.2.

Changed in qemu:
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Bug attachments

Remote bug watches

Bug watches keep track of this bug in other bug trackers.