[PATCH] D129735: [RISCV] Add new pass to transform undef to pseudo for vector values.

[Piyou Chen via Phabricator via llvm-commits]

BeMg marked 4 inline comments as done.
BeMg added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVRVVInitUndef.cpp:90-100
+  do {
+    switch (Super->getID()) {
+    // We only care vector register.
+    case RISCV::VRRegClassID:
+    case RISCV::VRM2RegClassID:
+    case RISCV::VRM4RegClassID:
+    case RISCV::VRM8RegClassID:
----------------
arsenm wrote:
> I don't understand the point of this function, getSuperClasses is already sorted by largest. You can just take the first? 
When this function take VRNoV0RegClassID as input. the getSuperClasses will return as following order.

AnyRegRegClassID 1
VMRegClassID 20
VRRegClassID 21 <- stop here
...

This patch only want those four RegClass as result.


================
Comment at: llvm/lib/Target/RISCV/RISCVRVVInitUndef.cpp:105
+
+bool RISCVInitUndef::isVectorRegClass(const Register R) {
+  return RISCV::VRRegClass.hasSubClassEq(MRI->getRegClass(R)) ||
----------------
arsenm wrote:
> Could this use the new getPhysRegBaseClass?
Could this hook use for virtual register? It seem only for physical register but this pass run before register allocation.


================
Comment at: llvm/lib/Target/RISCV/RISCVRVVInitUndef.cpp:201
+  VRegInfo *VRegInfos = new VRegInfo[NumVirtRegs];
+  Changed |= DDL.getSubRegisterLaneBitInfo(MF, VRegInfos);
+
----------------
craig.topper wrote:
> Isn't this calculating for the entire function? But handleSubReg is called for individual instructions. So we'll be recomputing information right?
Yes, you're right. we don't need recompute this info for each instruction. It call only once now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129735