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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

This could certainly use some new MIR tests. I didn't look super closely but I'm not sure you're correctly handling undef vs. not-undef subreg defs



================
Comment at: llvm/lib/Target/RISCV/RISCVRVVInitUndef.cpp:53
+  llvm::SmallPtrSet<MachineInstr *, 4> Seen;
+  std::map<MachineInstr *, LaneBitmask> PHINodeLaneBitRecord;
+
----------------
DenseMap


================
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:
----------------
I don't understand the point of this function, getSuperClasses is already sorted by largest. You can just take the first? 


================
Comment at: llvm/lib/Target/RISCV/RISCVRVVInitUndef.cpp:105
+
+bool RISCVInitUndef::isVectorRegClass(const Register R) {
+  return RISCV::VRRegClass.hasSubClassEq(MRI->getRegClass(R)) ||
----------------
Could this use the new getPhysRegBaseClass?


================
Comment at: llvm/lib/Target/RISCV/RISCVRVVInitUndef.cpp:107
+  return RISCV::VRRegClass.hasSubClassEq(MRI->getRegClass(R)) ||
+         RISCV::VRM2RegClass.hasSubClassEq(MRI->getRegClass(R)) ||
+         RISCV::VRM4RegClass.hasSubClassEq(MRI->getRegClass(R)) ||
----------------
Don't call getRegClass for each use


================
Comment at: llvm/lib/Target/RISCV/RISCVRVVInitUndef.cpp:141
+  Register Reg = Inst->getOperand(0).getReg();
+  if (!Register::isVirtualRegister(Reg))
+    return false;
----------------
Reg.isVirtual()


================
Comment at: llvm/lib/Target/RISCV/RISCVRVVInitUndef.cpp:200
+  unsigned NumVirtRegs = MRI->getNumVirtRegs();
+  VRegInfo *VRegInfos = new VRegInfo[NumVirtRegs];
+  Changed |= DDL.getSubRegisterLaneBitInfo(MF, VRegInfos);
----------------
unique_ptr


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129735