Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Target/RISCV/RISCVInstrInfoV.td | ||
---|---|---|
988 | "If index EEW is greater than XLEN, an illegal instruction exception is raised." We could collect all AMO instructions with EEW = 64 into one place and use "Predicates = [IsRV64]" to ensure XLEN >= EEW. |
llvm/lib/Target/RISCV/RISCV.td | ||
---|---|---|
152 | Zvamo should imply 'A' extension and 'V' extension. def FeatureExtZvamo : SubtargetFeature<"experimental-zvamo", "HasStdExtZvamo", "true", "'Zvamo'(Vector AMO Operations)", [FeatureStdExtA, FeatureStdExtV]>; | |
llvm/lib/Target/RISCV/RISCVInstrInfoV.td | ||
984 | Zvamo should imply 'A' extension and 'V' extension. So, use HasStdExtZvamo is enough. | |
1029 | Ditto. | |
llvm/test/MC/RISCV/rvv/zvamo.s | ||
2 | --mattr=+experimental-zvamo is enough. | |
16 | instruction requires the following: 'Zvamo'(Vector AMO Operations) |
I confused the relationship of sub-extensions before. Sorry for that.
There are some discussions[0] about the relationship between vector sub-extensions. It seems that
V-extension implies Zvamo + Zvlsseg
Zvqmac, Zvamo and Zvlsseg are stand alone sub extensions. That is, Zvamo does not imply V extension.
Zvqmac has no implied extensions.
Zvamo implies A-extension.
Zvlsseg has no implied extensions.
Do you agree with that?
Zvamo implies A-extension.
I think it's require instead of imply?
RVV Spec 8. Vector AMO Operations:
If vector AMO instructions are supported, then the scalar Zaamo instructions (atomic operations from the standard A extension) must be present.
So, let me summarize:
Zvlsseg has no implied extensions
Zvqmac has no implied extensions
Zvamo required A extension ? If so, is it suitable that use -mattr=+a, +experimental-zvamo to enable the zvamo extension?
Currently, FeatureStdExtV is the base V instructions. So, I will suggest the following relationships
Zvamo -> V (Does Zvamo -> A? I create an issue about it. https://github.com/riscv/riscv-v-spec/issues/555)
Zvqmac -> V
Zvlsseg -> V
The implementation looks like
def FeatureExtZvamo : SubtargetFeature<"experimental-zvamo", "HasStdExtZvamo", "true", "'Zvamo'(Vector AMO Operations)", [FeatureStdExtV]>; def FeatureExtZvqmac : SubtargetFeature<"experimental-zvqmac", "HasStdExtZvqmac", "true", "'Zvqmac' (Vector Quad-Widening Integer Multiply-Add Instructions)", [FeatureStdExtV]>; def FeatureStdExtZvlsseg : SubtargetFeature<"experimental-zvlsseg", "HasStdExtZvlsseg", "true", "'Zvlsseg' (Vector segment load/store instructions)”, [FeatureStdExtV]>;
In the future, “current V” may be changed to Zvbase in the specification. And “new V” will imply Zvbase + Zvamo + Zvlsseg. As described in https://github.com/riscv/riscv-v-spec/issues/547#issuecomment-670667964.
llvm/lib/Target/RISCV/RISCV.td | ||
---|---|---|
161 | There is no conclusion from 'V' specification. I think we could be conservative as Kito said. Zvamo does not imply 'A'. Please remove FeatureStdExtA from the list. Sorry for that. | |
llvm/lib/Target/RISCV/RISCVInstrInfoV.td | ||
984 | Add HasStdExtA. | |
1022 | Add HasStdExtA. | |
llvm/test/MC/RISCV/rvv/zvamo.s | ||
3 | --mattr=+a,experimental-zvamo |
Just a couple of nits, but otherwise it LGTM.
llvm/lib/Target/RISCV/RISCVInstrFormatsV.td | ||
---|---|---|
330 | Redundant space at the end. | |
llvm/lib/Target/RISCV/RISCVSchedRocket32.td | ||
20 | Rebase on master to include HasStdExtZvlsseg. | |
llvm/lib/Target/RISCV/RISCVSchedRocket64.td | ||
19 | Rebase on master to include HasStdExtZvlsseg. |
Zvamo should imply 'A' extension and 'V' extension.