[PATCH] D152421: [RISCV] Introduce the concept of DLEN(data path width) into getLMULCost.

Mon Jun 12 09:51:41 PDT 2023

craig.topper added inline comments.

================
Comment at: llvm/lib/Target/RISCV/RISCVFeatures.td:798

+def TuneDLenFactor2
+   : SubtargetFeature<"dlen-factor-2", "DLenFactor2", "true",
----------------
reames wrote:
> Can you add a comment here which defines DLEN?  
> 
> As an organizing principle, I was a bit surprised by the factor description.  I was expecting something more along the lines of VLEN (zvlNb) where we have specific sizes and then computed the factor from VLEN/DLEN if needed.  Not objecting, just commenting.
Having it as a factor removes the possibility of DLEN > VLEN. It also avoids having one number come from march and one from mtune.

SiFive has 2 CPUs, x280 and p270l, base on the same microarchitecture with different VLEN and DLEN but the factor is 2 for both.

================
Comment at: llvm/lib/Target/RISCV/RISCVFeatures.td:822
+                                    TuneShortForwardBranchOpt,
+                                    TuneDLenFactor2]>;

----------------
reames wrote:
> The default tuning is used for more than the x280, are you sure you want this placement?
We have 2 vector CPUs based on the 7 series microarchitecture. We just haven't put the other in LLVM. They both have factor of 2. So that's what I was thinking when I put this here, but I think I'll move it to x280.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152421