[RISCV][NFC] Mark rs1 in most memory instructions as memory operand.

[dybv-sc]

Summary

Marking rs1 (memory offset base) as memory operand provides additional
semantic value to this operand that can be used by different tools
(e.g. llvm-exegesis).

This change does not affect neigther Isel nor assembler. However it
required some tweaks in tablegen compressed inst emmiter.

[jrtc27]

llvm/lib/Target/RISCV/RISCVInstrInfo.td

131–135

Or change the RegisterOperand<GPR> to MemOperand<GPR>

140–146

You're missing a bunch of whitespace before tokens

144

I don't think we want an Operand suffix on these, we don't for other things

509

So what about all the immediate offsets? OPERAND_MEMORY surely isn't very useful without knowing the offset? It just tells you the instruction is touching memory somewhere in the +/- 2 KiB region centred around that register.

[dybv-sc]

llvm/lib/Target/RISCV/RISCVInstrInfo.td

509

Yes, it is an issue. But there is really not much can be done for immediate. It is not really possible to change it's operand type to OPERAND_MEMORY because the operand type defines their size and signness (and reg operands have their properties defined in RegClass not in type, so it's safe to change their type).
However I imagine it to be enough for tools, because I want them to know following: instruction is explicit load/store (mayStore or mayLoad == true is not enough) and their address is based on specific operand marked memory operand(and maybe some other input operands). For example, it may help in llvm-exegesis to produce serial execution for load/store instructions(we can ignore immediate offset here if we assume it to be fixed specific value).

[dybv-sc]

• Removed Operand suffixes
• Added whitespaces between definitons

[dybv-sc]

ping

[dybv-sc]

I want to ping on this again

[craig.topper]

LGTM

[jrtc27]

llvm/lib/Target/RISCV/RISCVInstrInfo.td

131–135

: needs a space before it (repeated multiple times below)

[dybv-sc]

Fixed formatting issues.

[dybv-sc]

rebased

[dybv-sc]

All fixed now. Ping for review.

[reames]

In D136847#3930781, @dybv-sc wrote:

All fixed now. Ping for review.

Why? Craig has given an LGTM and later comments were style only. Our usual take is that LGTM still holds and you can land.

(I have not looked at this patch at all, just making a process comment.)

[dybv-sc]

Thanks! I didn't know that.
I will rebase and ask for merge.

[dybv-sc]

Rebased and resolved merge conflicts.