[PATCH] D76113: [ExpandMemCmp] Correctly set alignment of generated loads

[Clement Courbet via Phabricator via llvm-commits]

courbet added inline comments.


================
Comment at: llvm/lib/CodeGen/ExpandMemCmp.cpp:286
         OffsetBytes);
+    LhsAlign = Align(GreatestCommonDivisor64(LhsAlign.value(), OffsetBytes));
+    RhsAlign = Align(GreatestCommonDivisor64(RhsAlign.value(), OffsetBytes));
----------------
Because alignments are always powers of two, assuming n<m, `gcd(2^n, 2^m)` is the same as `2^n * gcd(1, 2^(m-n))`, or `2^n`, i.e. `min(2^n, 2^m)`. So please use `Align commonAlignment(Align A, uint64_t Offset)` from Alignment.h


================
Comment at: llvm/test/CodeGen/AArch64/bcmp-inline-small.ll:19
 ; CHECKN-NEXT: ldur x
-; CHECKS:      ldr  x
-; CHECKS-NEXT: ldr  x
-; CHECKS-NEXT: ldr  w
-; CHECKS-NEXT: ldr  w
-; CHECKS-NEXT: ldrh w
-; CHECKS-NEXT: ldrh w
-; CHECKS-NEXT: ldrb w
-; CHECKS-NEXT: ldrb w
+; CHECKS-30:      ldrb
 }
----------------
aqjune wrote:
> courbet wrote:
> > I'm not sure what this checks exactly.
> > Can you expand the checked pattern ? It would make it clearer what we're looking for.
> It checks whether ldrb is repeated 30 times (for reading 15 bytes of %s1 and %s2 each).
> I think expanding this would be a bit verbose; I attach the output asm file.{F11576881}
ping


================
Comment at: llvm/test/Transforms/ExpandMemCmp/X86/memcmp.ll:25
+
+define i32 @cmp2_align2(i8* nocapture readonly align 2 %x, i8* nocapture readonly align 2 %y)  {
+; ALL-LABEL: @cmp2_align2(
----------------
Please submit the base tests to show the diffs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76113