[PATCH] D137785: [PowerPC][GISel] Add initial GlobalISel support for vector functions.

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/GISel/PPCRegisterBankInfo.cpp:62-68
 
+unsigned PPCRegisterBankInfo::copyCost(const RegisterBank &Dst,
+                                           const RegisterBank &Src,
+                                           unsigned Size) const {
+  // TODO: This function still requires implementation.
+  return RegisterBankInfo::copyCost(Dst, Src, Size);
+}
----------------
Why bother with this then?


================
Comment at: llvm/lib/Target/PowerPC/GISel/PPCRegisterBankInfo.cpp:154-155
+    unsigned SrcSize = SrcTy.getSizeInBits();
+    assert((DstSize == SrcSize) && "Can only bitcast when the source and "
+                                   "destination bit sizes are the same!");
+
----------------
Don't bother asserting on things the verifier checks 


================
Comment at: llvm/lib/Target/PowerPC/GISel/PPCRegisterBankInfo.cpp:158
+    bool DstIsGPR = !DstTy.isVector() && DstTy.getSizeInBits() <= 64;
+    bool SrcIsGPR = !SrcTy.isVector() && SrcTy.getSizeInBits() <= 64;
+    // TODO: Currently, only vector and GPR register banks are handled.
----------------
Don't need to bother checking each size 


================
Comment at: llvm/lib/Target/PowerPC/GISel/PPCRegisterBankInfo.cpp:167
+        getCopyMapping(DstRB.getID(), SrcRB.getID(), DstSize),
+        Opc == TargetOpcode::G_BITCAST ? 2 : 1);
+  }
----------------
There's no other opcode here 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137785