[X86] Use llvm::Align for passing the alignment

[pengfei]

Summary

This should be a typo in emitConstantSizeRepmov. Both its caller and
callee store the alignment in a 64-bit variables, no reason to truncate
it to 32-bit. It results in alignment turns into 0 when larger than
0x100000000.

Fixes #61348

[RKSimon]

llvm/lib/Target/X86/X86SelectionDAGInfo.cpp

208

Can we use llvm::Align instead?

[pengfei]

Address review comment.

[RKSimon]

LGTM - but please add the PR61348 test case as well

llvm/lib/Target/X86/X86SelectionDAGInfo.cpp

185–186

Probably worth converting this to Align Alignment as well?

[pengfei]

Convert one more uint64_t Align into Align Alignment.

In D145863#4188386, @RKSimon wrote:

LGTM - but please add the PR61348 test case as well

I hesitate to add PR61348 test case for 3 reasons,

1. Changing unsigned/uint64_t to Align is purely an NFC. No tests required;
2. The origin problem can be reproduced only in assertion mode. But it doesn't make much sense to add a REQUIRES: asserts for it;
3. The IR shouldn't generate such a large alignment. It's not reliable to test something based on it;

Let me know if you still think we should add the test case.

[RKSimon]

We have enough examples of tests with `REQUIRES: asserts``` in llvm-project\llvm\test\CodeGen\X86 that I don't think its unusual

[pengfei]

Add test case.

[RKSimon]

LGTM - cheers

llvm/lib/Target/X86/X86SelectionDAGInfo.cpp

static SDValue emitRepmovsB(const X86Subtarget &Subtarget, SelectionDAG &DAG,
                             const SDLoc &dl, SDValue Chain, SDValue Dst,
                             SDValue Src, uint64_t Size) {
   return emitRepmovs(Subtarget, DAG, dl, Chain, Dst, Src,
                      DAG.getIntPtrConstant(Size, dl), MVT::i8);
 }
 
 /// Returns the best type to use with repmovs depending on alignment.
 static MVT getOptimalRepmovsType(const X86Subtarget &Subtarget,
-                                 uint64_t Align) {
+                                 Align Alignment) {
+  uint64_t Align = Alignment.value();
   assert((Align != 0) && "Align is normalized");
   assert(isPowerOf2_64(Align) && "Align is a power of 2");
   switch (Align) {
   case 1:
     return MVT::i8;
   case 2:
     return MVT::i16;
   case 4:
     return MVT::i32;
   default:
     return Subtarget.is64Bit() ? MVT::i64 : MVT::i32;
   }
 }
 
 /// Returns a REP MOVS instruction, possibly with a few load/stores to implement
 /// a constant size memory copy. In some cases where we know REP MOVS is
 /// inefficient we return an empty SDValue so the calling code can either
 /// generate a load/store sequence or call the runtime memcpy function.
 static SDValue emitConstantSizeRepmov(
     SelectionDAG &DAG, const X86Subtarget &Subtarget, const SDLoc &dl,
     SDValue Chain, SDValue Dst, SDValue Src, uint64_t Size, EVT SizeVT,
-    unsigned Align, bool isVolatile, bool AlwaysInline,
+    Align Alignment, bool isVolatile, bool AlwaysInline,
     MachinePointerInfo DstPtrInfo, MachinePointerInfo SrcPtrInfo) {
 
   /// TODO: Revisit next line: big copy with ERMSB on march >= haswell are very
   /// efficient.
   if (!AlwaysInline && Size > Subtarget.getMaxInlineSizeThreshold())
     return SDValue();
 
   /// If we have enhanced repmovs we use it.
   if (Subtarget.hasERMSB())
     return emitRepmovsB(Subtarget, DAG, dl, Chain, Dst, Src, Size);
 
   assert(!Subtarget.hasERMSB() && "No efficient RepMovs");
   /// We assume runtime memcpy will do a better job for unaligned copies when
   /// ERMS is not present.
-  if (!AlwaysInline && (Align & 3) != 0)
+  if (!AlwaysInline && (Alignment.value() & 3) != 0)
     return SDValue();
 
-  const MVT BlockType = getOptimalRepmovsType(Subtarget, Align);
+  const MVT BlockType = getOptimalRepmovsType(Subtarget, Alignment);
   const uint64_t BlockBytes = BlockType.getSizeInBits() / 8;
   const uint64_t BlockCount = Size / BlockBytes;
   const uint64_t BytesLeft = Size % BlockBytes;
   SDValue RepMovs =
       emitRepmovs(Subtarget, DAG, dl, Chain, Dst, Src,
                   DAG.getIntPtrConstant(BlockCount, dl), BlockType);
 
   /// RepMov can process the whole length.
static SDValue emitConstantSizeRepmov(
   Results.push_back(RepMovs);
   unsigned Offset = Size - BytesLeft;
   EVT DstVT = Dst.getValueType();
   EVT SrcVT = Src.getValueType();
   Results.push_back(DAG.getMemcpy(
       Chain, dl,
       DAG.getNode(ISD::ADD, dl, DstVT, Dst, DAG.getConstant(Offset, dl, DstVT)),
       DAG.getNode(ISD::ADD, dl, SrcVT, Src, DAG.getConstant(Offset, dl, SrcVT)),
-      DAG.getConstant(BytesLeft, dl, SizeVT), llvm::Align(Align), isVolatile,
+      DAG.getConstant(BytesLeft, dl, SizeVT), Alignment, isVolatile,
       /*AlwaysInline*/ true, /*isTailCall*/ false,
       DstPtrInfo.getWithOffset(Offset), SrcPtrInfo.getWithOffset(Offset)));
   return DAG.getNode(ISD::TokenFactor, dl, MVT::Other, Results);
 }
 
 SDValue X86SelectionDAGInfo::EmitTargetCodeForMemcpy(
     SelectionDAG &DAG, const SDLoc &dl, SDValue Chain, SDValue Dst, SDValue Src,
     SDValue Size, Align Alignment, bool isVolatile, bool AlwaysInline,
const X86Subtarget &Subtarget =
       DAG.getMachineFunction().getSubtarget<X86Subtarget>();
 
   // If enabled and available, use fast short rep mov.
   if (UseFSRMForMemcpy && Subtarget.hasFSRM())
     return emitRepmovs(Subtarget, DAG, dl, Chain, Dst, Src, Size, MVT::i8);
 
   /// Handle constant sizes,
   if (ConstantSDNode *ConstantSize = dyn_cast<ConstantSDNode>(Size))
-    return emitConstantSizeRepmov(
-        DAG, Subtarget, dl, Chain, Dst, Src, ConstantSize->getZExtValue(),
-        Size.getValueType(), Alignment.value(), isVolatile, AlwaysInline,
-        DstPtrInfo, SrcPtrInfo);
+    return emitConstantSizeRepmov(DAG, Subtarget, dl, Chain, Dst, Src,
+                                  ConstantSize->getZExtValue(),
+                                  Size.getValueType(), Alignment, isVolatile,
+                                  AlwaysInline, DstPtrInfo, SrcPtrInfo);
 
   return SDValue();
 }

llvm/test/CodeGen/X86/pr61348.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 2
+; REQUIRES: asserts
+; RUN: llc < %s -mtriple=x86_64-- | FileCheck %s
+
+define i32 @PR61348() optsize {
+; CHECK-LABEL: PR61348:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    movl $3, %ecx
+; CHECK-NEXT:    xorl %edi, %edi
+; CHECK-NEXT:    xorl %esi, %esi
+; CHECK-NEXT:    rep;movsq (%rsi), %es:(%rdi)
+; CHECK-NEXT:    movb 30, %al
+; CHECK-NEXT:    movb %al, 30
+; CHECK-NEXT:    movzwl 28, %eax
+; CHECK-NEXT:    movw %ax, 28
+; CHECK-NEXT:    movl 24, %eax
+; CHECK-NEXT:    movl %eax, 24
+; CHECK-NEXT:    xorl %eax, %eax
+; CHECK-NEXT:    retq
+  tail call void @llvm.memcpy.p0.p0.i64(ptr align 1 null, ptr align 1 null, i64 31, i1 true)
+  ret i32 0
+}
+declare void @llvm.memcpy.p0.p0.i64(ptr noalias nocapture writeonly, ptr noalias nocapture readonly, i64, i1 immarg)