[PATCH] D154142: [llvm-mca][RISCV] Add RISCV-SEW instrument

[Michael Maitland via Phabricator via llvm-commits]

michaelmaitland added inline comments.


================
Comment at: llvm/test/tools/llvm-mca/RISCV/different-sew-instruments.s:8
+vdiv.vv v8, v8, v12
+vsetvli zero, a0, e64, m1, tu, mu
+# LLVM-MCA-RISCV-SEW E64
----------------
myhsu wrote:
> michaelmaitland wrote:
> > myhsu wrote:
> > > is there any specific reason to test with the same LMUL here?
> > We need a LMUL instrument since the vdiv.vv instruction depends on LMUL and SEW. I chose to use the same LMUL to show that the MCA reports changed based on SEW changing.
> > 
> > The reason we need to provide an LMUL to test SEW is because vdiv.vv depends on both LMUL and SEW. We could not resolve the correct SchedClassID without both. I thought about having a LMUL-SEW instrument instead of a SEW instrument, but it becomes trickier to replace an active LMUL instrument with the LMUL from the LMUL-SEW instrument. It was simpler to just use two instruments. I am open to improving this in the future.
> >  the same LMUL to show that the MCA reports changed based on SEW changing.
> 
> Ahh that makes sense, thanks
> 
> > We could not resolve the correct SchedClassID without both.
> 
> Does the current implementation //effectively// falls back to a default LMUL? If that's the case I think it's good enough as a baseline. That said, having a LMUL-SEW instrument would definitely be a nice addition.
It falls back to the “WorstCase” behavior. We have test coverage of no lmul nor sew provided, but I’ll add a test  that gives coverage of lmul but no sew for an instruction that needs sew + lmul. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154142