[PATCH] D86883: [X86] Add support for using fast short rep mov for memcpy lowering.

[Hiroshi Yamauchi via Phabricator via llvm-commits]

yamauchi added inline comments.


================
Comment at: llvm/lib/Target/X86/X86SelectionDAGInfo.cpp:198
   SDValue InFlag;
-  Chain = DAG.getCopyToReg(Chain, dl, CX, Size, InFlag);
+  Chain = DAG.getCopyToReg(
+      Chain, dl, CX,
----------------
yamauchi wrote:
> craig.topper wrote:
> > yamauchi wrote:
> > > craig.topper wrote:
> > > > Is this needed because we're no longer calling this with just constants we fixed the size of?
> > > Do you mean, "is the getCopyToReg call needed"? The size needs to be loaded into the RCX/ECX register for the rep movs instruction and the callers aren't doing that. So, it seems yes. Did you mean in a different way?
> > I was asking why the getZExtOrTrunc was added. do we create memcpy's where the size type isn't the same size as the pointer?
> Without the getZExtOrTrunc, I get this fatal error, for example,
> 
> Cannot copy EAX to RCX
> fatal error: error in backend: Cannot emit physreg copy instruction
> 
> which is coming from X86InstrInfo::copyPhysReg.
> 
> Re: "do we create memcpy's where the size type isn't the same size as the pointer?" yes, it seems so. 
The type mismatch is coming from here:

```
static SDValue CreateCopyOfByValArgument(SDValue Src, SDValue Dst,
                                         SDValue Chain, ISD::ArgFlagsTy Flags,
                                         SelectionDAG &DAG, const SDLoc &dl) {
  SDValue SizeNode = DAG.getConstant(Flags.getByValSize(), dl, MVT::i32);
  ...
```

I'm going to fix that instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86883