[PATCH] D145863: [X86][NFC] Use llvm::Align for passing the alignment

Phoebe Wang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 13 05:28:57 PDT 2023


pengfei updated this revision to Diff 504603.
pengfei marked an inline comment as done.
pengfei added a comment.

Convert one more `uint64_t Align` into `Align Alignment`.

In D145863#4188386 <https://reviews.llvm.org/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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145863

Files:
  llvm/lib/Target/X86/X86SelectionDAGInfo.cpp


Index: llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
===================================================================
--- llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
+++ llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
@@ -182,7 +182,8 @@
 
 /// 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) {
@@ -204,7 +205,7 @@
 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
@@ -219,10 +220,10 @@
   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;
@@ -251,7 +252,7 @@
       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);
@@ -281,10 +282,10 @@
 
   /// 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();
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D145863.504603.patch
Type: text/x-patch
Size: 2933 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230313/cffc7be9/attachment.bin>


More information about the llvm-commits mailing list