Fence test has poor coverage

[allenjbaum]

Fence has an optional .TSO extension. The fm field of the instruction is normally 0000, but is 1000 for the .TSO extension (specifically with P=0011 and S=0011). The instruction description says that other encodings are reserved -- but then says that reserved values should be ignored, which is different than normal reserved fields which tests avoid because we can't count on what happens.

So, the fence.01.S test needs to expanded to at least have a walking ones test for the fm field.

[pawks]

Setting the fm field for a fence instruction does not seem to be supported by gcc. Currently the only 2 versions of fence which compile are fence [rw],[rw] or fence.tso. Even using fence.tso rw,rw throws an illegal operand error on gcc.

The spec says -

Base implementations shall treat all such reserved configurations as normal fences with fm=0000, and standard software shall use only non-reserved configurations.

So I think only the fence.tso variant is missing from the coverage and the tests.

[allenjbaum]

Ooh - that's interesting. Let's discuss this in the meeting. That's a bug on the toolchain.

On Thu, Aug 26, 2021 at 4:19 AM S Pawan Kumar @.***> wrote:

Setting the fm field for a fence instruction does not seem to be supported by gcc. Currently the only 2 versions of fence which compile are fence [rw],[rw] or fence.tso. Even using fence.tso rw,rw throws an illegal operand error on gcc.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/riscv/riscv-arch-test/issues/203#issuecomment-906316510, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJWZHEJ2TF27BM6KAITT6YPLVANCNFSM5CXHY4LA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

[aswaterman]

There's no reason to wait for an assembler change. Just use the .word directive and manually assemble the instruction.

In fact, I'm not sure the assembler change will ever come; the assembler maintainers might (legitimately) balk at adding support for assembling invalid instructions. .word will always work.

[allenjbaum]

Let's go with Andrew's suggestion then. He also pointed out that for SFENCE.VMA and friends, the MSBs of rs2 are reserved for future use, should be zeroed by software, and should be ignored by current implementations. We don't have tests for that yet, but we need to note it somewhere for when we do.

[allenjbaum]

Resolve to write more tests; bring up to dev partners, and close when tests are merged.

[allenjbaum]

There is a lot of overlap here with issue #119

[allenjbaum]

The syntax for a fence instruction with arbitrary params is: .insn i 0x0F, 0, rd, rs1, (signed_fm<<8) | (pred<<4) | succ ) where legal values of fm are 0, -8,

The syntax for a fence.i instruction with arbitrary params is: .insn i 0x0F, 1, rd, rs1, signed_imm12

The syntax for an sfence.vma instruction with arbitrary params is: .insn r 0x73, 0, 0x09, 0, rs1, rs2