[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:141
   MachineInstrBuilder &MIB;
+  uint64_t StackSize;
+  const DataLayout &DL;
----------------
I am  not sure why you made this members as public but i believe that they should be private as no one outside the class scope should change them. It is especially important for non const members.


================
Comment at: test/CodeGen/X86/GlobalISel/callingconv.ll:123
+
+declare void @trivial_callee()
+define void @test_trivial_call() {
----------------
oren_ben_simhon wrote:
> Could you please add tests that exercise arguments and return values that are spitted (like structures)?
Where are the tests for returned/passed structures?


================
Comment at: test/CodeGen/X86/GlobalISel/callingconv.ll:3
 ; RUN: llc -mtriple=i386-linux-gnu -mattr=+sse2  -global-isel -verify-machineinstrs < %s -o - | FileCheck %s --check-prefix=ALL --check-prefix=X32 --check-prefix=X32_GISEL
-; RUN: llc -mtriple=i386-linux-gnu -mattr=+sse2               -verify-machineinstrs < %s -o - | FileCheck %s --check-prefix=ALL --check-prefix=X32 --check-prefix=X32_ISEL
 ; RUN: llc -mtriple=x86_64-linux-gnu             -global-isel -verify-machineinstrs < %s -o - | FileCheck %s --check-prefix=ALL --check-prefix=X64 --check-prefix=X64_GISEL
 
----------------
Remove --check-prefix=X64_GISEL which is not needed.



https://reviews.llvm.org/D34602