[PATCH] D83175: [X86] Fix a bug that when lowering byval argument

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:3774
+  // creating a temporary stack slot, again.
+  if (Flags.isByVal() && !hasCopy)
     return CreateCopyOfByValArgument(Arg, PtrOff, Chain, Flags, DAG, dl);
----------------
LuoYuanke wrote:
> Let me check below 2 rule is right or not.
> 1. On linux when the byval attribute is set, it indicate copy the value that point by the pointer to the parameter stack.
> 2. On window when the byval attribute is set, it indicate that allocate temporary object in caller, copy the value to the temporary, and store the temporary pointer (which point to the temporary object) to the parameter stack.
> 
> On linux, the VA.getLocInfo() is CCValAssign::Full, and on windows is the VA.getLocInfo() is CCValAssign::Indirect. 
> 
> So I think we can just check the  VA.getLocInfo(). If  VA.getLocInfo() is CCValAssign::Indirect, we can NOT copy object. Instead we just restore the pointer.
> `(Flags.isByVal() && VA.getLocInfo() != CCValAssign::Indirect)`
The hasCopy should just be isByVal with no inversion and the Flags.isByVal() should be removed and replaced with isByVal. isByVal replaced the version in Flags.

Or what Yuanke said might work.


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

https://reviews.llvm.org/D83175