[X86] Teach X86SelectionDAGInfo::EmitTargetCodeForMemcpy about GNUX32

[craig.topper]

Summary

In GNUX23, is64BitMode returns true, but pointers are 32-bits. So we shouldn't copy pointer values into RSI/RDI since the widths don't match.

Fixes PR38865 despite what the title says. I think the llvm_unreachable in the copyPhysReg code tricked the optimizer and made the fatal error trigger.

[efriedma]

test/CodeGen/X86/pr38865.ll

24

Shouldn't this be rep;movsl (%esi), (%edi), to get the right encoding?

[craig.topper]

test/CodeGen/X86/pr38865.ll

24

Are you saying we should be using the 0x67 adsize prefix?

[efriedma]

test/CodeGen/X86/pr38865.ll

24

Yes, to be consistent with other memory operations.

[craig.topper]

Use 0x67 address size prefix for x32. Don't disable movsq with x32.

To make the 0x67 work I had to add AdSize32 and AdSize64 to the CodeGenOnly instructions we use here. These control bits in TSFlags. The encoder will emit a 0x67 when the AdSize value is different than the current mode. I had to do it this way because these instructions aren't carrying any operands once they reach the MC layer so there is nothing else for the encoder to key off of.

I've also altered the Predicates to use IsLP64 and NotLP64 to choose the address size instead of just the mode.

Finally I altered the strings printed by the AsmWriter to explicitly mention the register names so that we will still generate a 0x67 prefix if the assembly is written to a text file and then fed to an assembler to generate a binary. I believe gcc would use an "addr32" prefix here, but I'm not sure our assembler supprots that.

[craig.topper]

I believe we have the same bug for memset as well, but I'd prefer to tackle that as a follow up since I don't have a test case yet.

[efriedma]

LGTM

lib/Target/X86/X86InstrCompiler.td

412 ↗

(On Diff #165000)

Probably should be Requires<[NotLP64, In64BitMode]>, for the sake of clarity.

lib/Target/X86/X86SelectionDAGInfo.cpp

SDValue X86SelectionDAGInfo::EmitTargetCodeForMemcpy(
   const MCPhysReg ClobberSet[] = {X86::RCX, X86::RSI, X86::RDI,
                                   X86::ECX, X86::ESI, X86::EDI};
   if (isBaseRegConflictPossible(DAG, ClobberSet))
     return SDValue();
 
   // If the target has enhanced REPMOVSB, then it's at least as fast to use
   // REP MOVSB instead of REP MOVS{W,D,Q}, and it avoids having to handle
   // BytesLeft.
+  bool Use64BitRegs = Subtarget.isTarget64BitLP64();
   if (!Subtarget.hasERMSB() && !(Align & 1)) {
     if (Align & 2)
       // WORD aligned
       Repeats.AVT = MVT::i16;
     else if (Align & 4)
       // DWORD aligned
       Repeats.AVT = MVT::i32;
     else
       // QWORD aligned
-      Repeats.AVT = Subtarget.is64Bit() ? MVT::i64 : MVT::i32;
+      Repeats.AVT = Use64BitRegs ? MVT::i64 : MVT::i32;
 
     if (Repeats.BytesLeft() > 0 &&
         DAG.getMachineFunction().getFunction().optForMinSize()) {
       // When agressively optimizing for size, avoid generating the code to
       // handle BytesLeft.
       Repeats.AVT = MVT::i8;
     }
   }
 
   SDValue InFlag;
-  Chain = DAG.getCopyToReg(Chain, dl, Subtarget.is64Bit() ? X86::RCX : X86::ECX,
+  Chain = DAG.getCopyToReg(Chain, dl, Use64BitRegs ? X86::RCX : X86::ECX,
                            DAG.getIntPtrConstant(Repeats.Count(), dl), InFlag);
   InFlag = Chain.getValue(1);
-  Chain = DAG.getCopyToReg(Chain, dl, Subtarget.is64Bit() ? X86::RDI : X86::EDI,
+  Chain = DAG.getCopyToReg(Chain, dl, Use64BitRegs ? X86::RDI : X86::EDI,
                            Dst, InFlag);
   InFlag = Chain.getValue(1);
-  Chain = DAG.getCopyToReg(Chain, dl, Subtarget.is64Bit() ? X86::RSI : X86::ESI,
+  Chain = DAG.getCopyToReg(Chain, dl, Use64BitRegs ? X86::RSI : X86::ESI,
                            Src, InFlag);
   InFlag = Chain.getValue(1);
 
   SDVTList Tys = DAG.getVTList(MVT::Other, MVT::Glue);
   SDValue Ops[] = { Chain, DAG.getValueType(Repeats.AVT), InFlag };
   SDValue RepMovs = DAG.getNode(X86ISD::REP_MOVS, dl, Tys, Ops);
 
   SmallVector<SDValue, 4> Results;

test/CodeGen/X86/pr38865.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s | FileCheck %s
+
+target datalayout = "e-m:e-p:32:32-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnux32"
+
+%struct.a = type { [65 x i32] }
+
+@c = global %struct.a zeroinitializer, align 4
+
+define void @e() nounwind {
+; CHECK-LABEL: e:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    pushq %rbx
+; CHECK-NEXT:    subl $528, %esp # imm = 0x210
+; CHECK-NEXT:    leal {{[0-9]+}}(%rsp), %ebx
+; CHECK-NEXT:    movl $c, %esi
+; CHECK-NEXT:    movl $260, %edx # imm = 0x104
+; CHECK-NEXT:    movl %ebx, %edi
+; CHECK-NEXT:    callq memcpy
+; CHECK-NEXT:    movl $65, %ecx
+; CHECK-NEXT:    movl %esp, %edi
+; CHECK-NEXT:    movl %ebx, %esi
+; CHECK-NEXT:    rep;movsl
+; CHECK-NEXT:    callq d
+; CHECK-NEXT:    addl $528, %esp # imm = 0x210
+; CHECK-NEXT:    popq %rbx
+; CHECK-NEXT:    retq
+entry:
+  %byval-temp = alloca %struct.a, align 8
+  %0 = bitcast %struct.a* %byval-temp to i8*
+  call void @llvm.memcpy.p0i8.p0i8.i32(i8* nonnull align 8 %0, i8* align 4 bitcast (%struct.a* @c to i8*), i32 260, i1 false)
+  call void @d(%struct.a* byval nonnull align 8 %byval-temp)
+  ret void
+}
+
+declare void @d(%struct.a* byval align 8) local_unnamed_addr #1
+
+declare void @llvm.memcpy.p0i8.p0i8.i32(i8* nocapture writeonly, i8* nocapture readonly, i32, i1)