[PATCH] D69372: [X86][VARARG] Avoid spilling xmm vararg arguments.

[Alexey Lapshin via Phabricator via llvm-commits]

avl marked 2 inline comments as done.
avl added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:3711
+        VarargXMMOps.push_back(ALVal);
+        VarargXMMOps.push_back(
+            DAG.getIntPtrConstant(FuncInfo->getThunkRegSaveFrameIndex(), dl));
----------------
rnk wrote:
> I haven't reviewed all of this code, but we have to find some way to refactor LowerCall. It was already poorly factored and long, but this is just too much complexity, too many lines of code. We need to find some way to separate concerns.
Ok, I would refactor it. Would it be OK if that refactoring would be a part of this patch ? Or Do I need to make it previously in separate patch ? 


================
Comment at: llvm/test/CodeGen/X86/musttail-varargs.ll:299-300
 
-; This thunk shouldn't require any spills and reloads, assuming the register
-; allocator knows what it's doing.
 
----------------
rnk wrote:
> This seems unfortunate. :( Does all this go away if you put the "thunk" attribute on it? Everything in this file is meant to be a test for universal thunks, so adding the attribute is reasonable.
yes. all this xmm save/restore code would go away if "thunk" is specified. I will add it to the test case. 

additionally, I would like to make separate patch which would NOT do this xmm store/restore if noimplicitfloat=false.  So that store/restore code is generated for only noimplicitfloat=true case. 

thus, I assume following patches would be done:

1. this patch. xmm stores/restores through phys regs would be generated for all _usual|_ thunks(not including universal thunks)
2. patch which will fix ABI breakage for noimplicitfloat case - D62639
3. do not generate xmm store/restore through phys regs for noimplicitfloat=false case.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69372