Acquire and release instructions have a bug

[AFOliveira]

Describe the bug
The bug is the aq/rl bit not correctly defined, its left empty in the encoding instead of being 0/1. This is due to my original parsing of instructions coming from riscv-opcodes db and not from the generated .json it creates.

I'm fixing this and I'll PR soon.

[AFOliveira]

Extending on this problem:

riscv-opcodes keeps acquire and release bits always as variable on atomic operations. This is compliant with the ISA standard description of this instructions:

However, LLVM is much more strict on these instructions, and separates them as, per example AMOADD.H - AQ/RL = 00 ---AMOADD.H.AQ - AQ/RL = 10 --- AMOADD.H.RL - AMOADD.H.RL = 01.

I think we could keep this as aq/rl variables for the purpose of the UDB being a DB and extend if we need, but since this caught my eye due to failing the #356 test, I want to ask for your opinion.

[ThinkOpenly]

There is more to this. The AMO instructions include the "aq" and "rl" specification in the mnemonics. Taking just amoadd.w as an example, it actually has four associated mnemonics:

• amoadd.w
• amoadd.w.aq
• amoadd.w.rl
• amoadd.w.aq.rl

[AFOliveira]

Then should we split the mnemonics from the variables?

From what I can grasp, it is only one instruction that has several mnemonics. I think this is a hard-line on SW vs HW view of instructions. While for software they can be considered many, I believe on HW they are only one. Would it make sense if we change the assembly to acommodate several mnemonics?

[lenary]

The problem is only partly the assembler, honestly.

The "orderings" are really another kind of operand that has been spliced onto the mnemonic - note how fence has ordering operands which aren't in the mnemonic, same goes for floating rounding modes which aren't in the mnemonic. In RISC-V, this is the only place where an assembler would need to slice up the first token to split a mnemonic (the instruction) from its operands.

Knowing when to do this slicing and when not to becomes hard, for an arbitrary mnemonic parsed from an assembly file.

I would highly suggest keeping them separate, I don't see the problem of them having identical pseudo-code, but with aq, rl parameters to that pseudocode set to constants.

This will make building a reference assembler from the riscv-unified-db much easier in the long run.

[dhower-qc]

@lenary, I agree with the vision. The question is if we can break the existing assembly that's out in the wild?

[ThinkOpenly]

The question is if we can break the existing assembly that's out in the wild?

I don't think we're talking about changes to assembly. That would be frought. My interpretation of the discussion is that we're talking about the UDB implementation, and how to avoid so much duplication to accommodate the additional mnemonics with very similar implementations. As in, can we parameterize the YAML realizations.

I think the answer is yes, and can be based on the same "layout" capability used by some CSR "families" like mhpmevent*.

[dhower-qc]

The "layout" flows would, I presume, generate separate YAML files for amoadd.w, amoadd.w.aq, amoadd.w.rl, and amoadd.w.aq.rl. From the perspective of a UDB-based assembler, doesn't that look like four different instructions, all with their own pseudocode? I thought @lenary was advocating that they all use a single YAML file, with the assembly transformed from:

amoadd.w.rl ...

to

amoadd.w ..., rl

[ThinkOpenly]

The "layout" flows would, I presume, generate separate YAML files for amoadd.w, amoadd.w.aq, amoadd.w.rl, and amoadd.w.aq.rl. From the perspective of a UDB-based assembler, doesn't that look like four different instructions, all with their own pseudocode?

Yes. Is there a way to "call out" from IDL to a "global" function that is parameterized? I'm not sure how to make that trivially visible to downstream users, even if so. hmm

a single YAML file, with the assembly transformed...

I don't see that as a viable option.