[PATCH] D27845: [SelectionDAG] Enable target specific vector scalarization of calls and returns

[Zoran Jovanovic via Phabricator via llvm-commits]

zoran.jovanovic accepted this revision.
zoran.jovanovic added a comment.
This revision is now accepted and ready to land.

LGTM with few nits.



================
Comment at: include/llvm/Target/TargetLowering.h:974
+
+  virtual MVT getRegisterTypeForCallingConv(LLVMContext &Context, EVT VT) const {
+    return getRegisterType(Context, VT);
----------------
Line exceeds 80 characters,


================
Comment at: include/llvm/Target/TargetLowering.h:981
+  /// integer register set.
+  virtual unsigned getNumRegistersForCallingConv(LLVMContext &Context, EVT VT) const {
+    return getNumRegisters(Context, VT);
----------------
Line exceeds 80 characters.


================
Comment at: include/llvm/Target/TargetLowering.h:987
+  /// type has the alignment requirement of another type.
+  virtual unsigned getABIAlignmentForCallingConv(Type * ArgTy, DataLayout DL) const {
+    return DL.getABITypeAlignment(ArgTy);
----------------
Line exceeds 80 characters.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:725
+
+    unsigned NumRegs =
+        IsABIRegCopy
----------------
Is it possible to save NumRegs value as a class member and use it in class member functions (e.g. getCopyToRegs)?


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:839
+                         ? TLI.getRegisterTypeForCallingConv(RegVTs[Value])
+                         : RegVTs[Value];
 
----------------
Is it possible to get undefined value for RegVTs[Value] if value of argument IsABIRegCopy is different from value used in class constructor?


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:1095
+    Result = RFV.getCopyFromRegs(DAG, FuncInfo, getCurSDLoc(), Chain, nullptr,
+                                 V, IsABIRegCopy);
     resolveDanglingDebugInfo(V, Result);
----------------
Is additional IsABIRegCopy parameter necessary?
Is it possible to use value passed to RegsForValue  constructor instead?



================
Comment at: lib/Target/Mips/MipsCCState.h:57
+  void
+  PreAnalyzeFormalArgumentsForVectorFloat(const SmallVectorImpl<ISD::InputArg> &Ins);
+
----------------
Line exceeds 80 characters.


================
Comment at: lib/Target/Mips/MipsCCState.h:59
+
+  void PreAnalyzeReturnForVectorFloat(const SmallVectorImpl<ISD::OutputArg> &Outs);
+
----------------
Line exceeds 80 characters.


================
Comment at: lib/Target/Mips/MipsCallingConv.td:42
+class CCIfOrigArgWasNotVectorFloat<CCAction A>
+    : CCIf<"!static_cast<MipsCCState *>(&State)->WasOriginalRetVectorFloat(ValNo)",
+           A>;
----------------
Line exceeds 80 characters.


================
Comment at: lib/Target/Mips/MipsISelLowering.cpp:2542
+    }
+  } else if (ValVT == MVT::i32 || (ValVT == MVT::f32 && AllocateFloatsInIntReg)) {
     Reg = State.AllocateReg(IntRegs);
----------------
Line exceeds 80 characters.


https://reviews.llvm.org/D27845