[PATCH] D34602: [GlobalISel][X86] Support call ABI.

[Oren Ben Simhon via Phabricator via llvm-commits]

oren_ben_simhon added inline comments.


================
Comment at: lib/Target/X86/X86CallLowering.cpp:77
+                       MachineInstrBuilder &MIB, CCAssignFn *AssignFn,
+                       const DataLayout &DL, const X86Subtarget &STI)
+      : ValueHandler(MIRBuilder, MRI, AssignFn), MIB(MIB), StackSize(0), DL(DL),
----------------
You don't realy need to pass the DataLayout.
You can get it from the MIRBuilder, something like this:

MachineFunction &MF = MIRBuilder.getMF();
auto &DL = MF.getDataLayout();

Same goes for STI.


================
Comment at: lib/Target/X86/X86CallLowering.cpp:84
+
+    LLT p0 = LLT::pointer(0, DL.getPointerSizeInBits(0));
+    LLT SType = LLT::scalar(DL.getPointerSizeInBits(0));
----------------
Consider calling DL.getPointerSizeOnBits(0) once and assign it to a variable 


================
Comment at: lib/Target/X86/X86CallLowering.cpp:109
+
+    // unsigned Alignment = DL.getABITypeAlignment(ArgVal->getType());
+    unsigned ExtReg = extendRegister(ValVReg, VA);
----------------
Please don't  submit commented code


================
Comment at: lib/Target/X86/X86CallLowering.cpp:122
+    if (!Info.IsFixed)
+      return true; // TODO: handle variadic function
+
----------------
Since you don't handle variadic functions, an assert should be added before returning true


https://reviews.llvm.org/D34602