[PATCH] D149495: [RISCV] Add support for V extension in SiFive7

[Wang Pengcheng via Phabricator via cfe-commits]

pcwang-thead accepted this revision.
pcwang-thead added a comment.

LGTM.
Though I don't like the way that we need to loop `MxList` again when defining scheduling model, I think this can be a good practice to define RVV scheduling model in current TableGen's grammar.



================
Comment at: llvm/lib/Target/RISCV/RISCVSchedSiFive7.td:15
+class SiFive7IsWorstCaseMX<string mx, list<string> MxList> {
+  string LLMUL = LargestLMUL<MxList>.r;
+  bit c = !eq(mx, LLMUL);
----------------
michaelmaitland wrote:
> pcwang-thead wrote:
> > I think I have fixed the issue that `defar` can't refer to template arguments in D148197. So `LMUL`, `SSEW` and other fields can be replaced with `defvar`s.
> A `defvar` statement defines a global variable. `SiFive7IsWorstCaseMX<mx, SchedMxList>.c` becomes invalid syntax since `c` is not a member of `SiFive7IsWorstCaseMX` now that `c` is global. If we did use `defvar` here and gave `c` a more unique name so that it could fit in the global space, then we would also need to complicate it by adding a `MXxxx_MxListxxx` suffix for all `mx` and `SchedMxList` pairs. 
> 
> I think we'd like to keep these as `Type Iden = Value;`. What do you think?
I mean:
```
class SiFive7IsWorstCaseMX<string mx, list<string> MxList> {
  defvar LLMUL = LargestLMUL<MxList>.r;
  bit c = !eq(mx, LLMUL);
}
```
`LLMUL` can be a local variable so that there is only one field `c`(which is the result of this class-based subroutine). We can keep only one field as result and change other fields to local variables. In this way, we know that `SiFive7IsWorstCaseMX` is a subroutine conventionally and can be replace with `function` in the future(if my patches is merged).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149495/new/

https://reviews.llvm.org/D149495