[CGP] Update MemIntrinsic alignment if possible

[arichardson]

Summary

Previously it was only being done if shouldAlignPointerArgs() returned
true, which right now is only true for ARM targets.
Updating the argument alignment attributes of memcpy/memset intrinsics
if the underlying object has larger alignment can be beneficial even
if CGP didn't increase alignment (as can be seen from the test changes),
so invert the loop and if condition.

[pengfei]

llvm/lib/CodeGen/CodeGenPrepare.cpp

2227–2233

Re-format it.

llvm/test/CodeGen/X86/memset-2.ll

13–23

Not sure if the use of movaps is a good idea considering the test in below file.

llvm/test/CodeGen/X86/memset64-on-x86-32.ll

18–55

Are they intent to use movl/q here?

llvm/test/Transforms/CodeGenPrepare/X86/memset_chk-simplify-nobuiltin.ll

14 ↗

(On Diff #461555)

No ATTR1 to match with it. Just removing the #1 instead?

[arichardson]

llvm/test/CodeGen/X86/memset-2.ll

13–23

I guess the real fix would be to not use null as the destination. I'm not sure what the test is actually checking, but I could change it to be an argument?

llvm/test/Transforms/CodeGenPrepare/X86/memset_chk-simplify-nobuiltin.ll

14 ↗

(On Diff #461555)

Yeah will regen with --scrub-attributes

[pengfei]

llvm/test/CodeGen/X86/memset-2.ll

13–23

How about change 0 to 1?
With a read on the history, I think the intention was to make sure no movups generated. See rG7998b1d6f and rGa048c83fe47

llvm/test/CodeGen/X86/memset64-on-x86-32.ll

56

Ditto here. Change 0 to 4?

[arichardson]

Address feedback

[arichardson]

llvm/test/CodeGen/X86/memset-2.ll

13–23

I've added an argument instead of using null as the base - that seems to restore the previous behaviour of calling memset.

llvm/test/CodeGen/X86/memset64-on-x86-32.ll

18–55

Will change the null argument to a function input - this mostly restores the old codegen.

[efriedma]

llvm/lib/CodeGen/CodeGenPrepare.cpp

2254–2255

Is this change putting this code inside the for loop? We don't need to run this more than once per call.

[arichardson]

address review comment

[efriedma]

LGTM

llvm/lib/CodeGen/CodeGenPrepare.cpp

if (optimizeInlineAsmInst(CI))
       return true;
   }
 
   // Align the pointer arguments to this call if the target thinks it's a good
   // idea
   unsigned MinSize;
   Align PrefAlign;
   if (TLI->shouldAlignPointerArgs(CI, MinSize, PrefAlign)) {
     for (auto &Arg : CI->args()) {
       // We want to align both objects whose address is used directly and
       // objects whose address is used in casts and GEPs, though it only makes
       // sense for GEPs if the offset is a multiple of the desired alignment and
       // if size - offset meets the size threshold.
       if (!Arg->getType()->isPointerTy())
         continue;
       APInt Offset(DL->getIndexSizeInBits(
                        cast<PointerType>(Arg->getType())->getAddressSpace()),
                    0);
       Value *Val = Arg->stripAndAccumulateInBoundsConstantOffsets(*DL, Offset);
       uint64_t Offset2 = Offset.getLimitedValue();
       if (!isAligned(PrefAlign, Offset2))
         continue;
       AllocaInst *AI;
       if ((AI = dyn_cast<AllocaInst>(Val)) && AI->getAlign() < PrefAlign &&
           DL->getTypeAllocSize(AI->getAllocatedType()) >= MinSize + Offset2)
         AI->setAlignment(PrefAlign);
       // Global variables can only be aligned if they are defined in this
       // object (i.e. they are uniquely initialized in this object), and
       // over-aligning global variables that have an explicit section is
       // forbidden.
       GlobalVariable *GV;
       if ((GV = dyn_cast<GlobalVariable>(Val)) && GV->canIncreaseAlignment() &&
           GV->getPointerAlignment(*DL) < PrefAlign &&
           DL->getTypeAllocSize(GV->getValueType()) >= MinSize + Offset2)
         GV->setAlignment(PrefAlign);
     }
-    // If this is a memcpy (or similar) then we may be able to improve the
-    // alignment
-    if (MemIntrinsic *MI = dyn_cast<MemIntrinsic>(CI)) {
-      Align DestAlign = getKnownAlignment(MI->getDest(), *DL);
-      MaybeAlign MIDestAlign = MI->getDestAlign();
-      if (!MIDestAlign || DestAlign > *MIDestAlign)
-        MI->setDestAlignment(DestAlign);
-      if (MemTransferInst *MTI = dyn_cast<MemTransferInst>(MI)) {
-        MaybeAlign MTISrcAlign = MTI->getSourceAlign();
-        Align SrcAlign = getKnownAlignment(MTI->getSource(), *DL);
-        if (!MTISrcAlign || SrcAlign > *MTISrcAlign)
-          MTI->setSourceAlignment(SrcAlign);
-      }
-    }
-  }
+  }
+  // alignment.
 
   // If we have a cold call site, try to sink addressing computation into the
   // cold block.  This interacts with our handling for loads and stores to
   // ensure that we can fold all uses of a potential addressing computation
   // into their uses.  TODO: generalize this to work over profiling data
   if (CI->hasFnAttr(Attribute::Cold) && !OptSize &&
       !llvm::shouldOptimizeForSize(BB, PSI, BFI.get()))
     for (auto &Arg : CI->args()) {

llvm/test/CodeGen/AMDGPU/lower-lds-struct-aa-memcpy.ll

 
 ;.
 ; CHECK: @[[LLVM_AMDGCN_KERNEL_TEST_LDS:[a-zA-Z0-9_$"\\.-]+]] = internal addrspace(3) global [[LLVM_AMDGCN_KERNEL_TEST_LDS_T:%.*]] undef, align 4
 ;.
 define protected amdgpu_kernel void @test(i8 addrspace(1)* nocapture %ptr.coerce) local_unnamed_addr #0 {
 ; GCN-LABEL: test:
 ; GCN:       ; %bb.0: ; %entry
 ; GCN-NEXT:    v_mov_b32_e32 v0, 0
-; GCN-NEXT:    ds_read_u8 v1, v0 offset:1
+; GCN-NEXT:    v_mov_b32_e32 v1, 2
+; GCN-NEXT:    ds_write_b8 v0, v1
 ; GCN-NEXT:    ds_read_u8 v2, v0 offset:2
+; GCN-NEXT:    ds_read_u16 v3, v0
 ; GCN-NEXT:    s_load_dwordx2 s[0:1], s[0:1], 0x24
-; GCN-NEXT:    v_mov_b32_e32 v3, 2
-; GCN-NEXT:    ds_write_b8 v0, v3
-; GCN-NEXT:    ds_write_b8 v0, v3 offset:4
 ; GCN-NEXT:    s_waitcnt lgkmcnt(0)
-; GCN-NEXT:    ds_write_b8 v0, v1 offset:5
 ; GCN-NEXT:    ds_write_b8 v0, v2 offset:6
-; GCN-NEXT:    v_mov_b32_e32 v1, 1
+; GCN-NEXT:    ds_write_b16 v0, v3 offset:4
+; GCN-NEXT:    v_cmp_eq_u16_sdwa s[2:3], v3, v1 src0_sel:BYTE_0 src1_sel:DWORD
+; GCN-NEXT:    v_cndmask_b32_e64 v1, 0, 1, s[2:3]
 ; GCN-NEXT:    global_store_byte v0, v1, s[0:1]
 ; GCN-NEXT:    s_endpgm
 ; CHECK-LABEL: @test(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[TMP0:%.*]] = getelementptr inbounds [[VEC_TYPE:%.*]], [[VEC_TYPE]] addrspace(3)* getelementptr inbounds ([[LLVM_AMDGCN_KERNEL_TEST_LDS_T:%.*]], [[LLVM_AMDGCN_KERNEL_TEST_LDS_T]] addrspace(3)* @llvm.amdgcn.kernel.test.lds, i32 0, i32 0), i32 0, i32 0, i32 0, i32 0, i32 0, i32 0
 ; CHECK-NEXT:    store i8 3, i8 addrspace(3)* [[TMP0]], align 4, !alias.scope !0, !noalias !3
 ; CHECK-NEXT:    [[TMP1:%.*]] = getelementptr inbounds [[VEC_TYPE]], [[VEC_TYPE]] addrspace(3)* getelementptr inbounds ([[LLVM_AMDGCN_KERNEL_TEST_LDS_T]], [[LLVM_AMDGCN_KERNEL_TEST_LDS_T]] addrspace(3)* @llvm.amdgcn.kernel.test.lds, i32 0, i32 2), i32 0, i32 0, i32 0, i32 0, i32 0, i32 0
 ; CHECK-NEXT:    [[TMP2:%.*]] = getelementptr inbounds [[VEC_TYPE]], [[VEC_TYPE]] addrspace(3)* getelementptr inbounds ([[LLVM_AMDGCN_KERNEL_TEST_LDS_T]], [[LLVM_AMDGCN_KERNEL_TEST_LDS_T]] addrspace(3)* @llvm.amdgcn.kernel.test.lds, i32 0, i32 0), i32 0, i32 0, i32 0, i32 0, i32 0, i32 0

llvm/test/CodeGen/X86/mcu-abi.ll

entry:
     %r = add i64 %iext, %d2i
     %rd = bitcast i64 %r to double
     ret double %rd
 }
 
 define void @ret_large_struct(ptr noalias nocapture sret(%struct.st12_t) %agg.result, ptr byval(%struct.st12_t) nocapture readonly align 4 %r) #0 {
 ; CHECK-LABEL: ret_large_struct:
 ; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    pushl %edi
 ; CHECK-NEXT:    pushl %esi
-; CHECK-NEXT:    movl %eax, %esi
-; CHECK-NEXT:    leal {{[0-9]+}}(%esp), %edx
-; CHECK-NEXT:    movl $48, %ecx
-; CHECK-NEXT:    calll memcpy
-; CHECK-NEXT:    movl %esi, %eax
+; CHECK-NEXT:    leal {{[0-9]+}}(%esp), %esi
+; CHECK-NEXT:    movl $12, %ecx
+; CHECK-NEXT:    movl %eax, %edi
+; CHECK-NEXT:    rep;movsl (%esi), %es:(%edi)
 ; CHECK-NEXT:    popl %esi
+; CHECK-NEXT:    popl %edi
 ; CHECK-NEXT:    retl
 entry:
   call void @llvm.memcpy.p0.p0.i32(ptr %agg.result, ptr %r, i32 48, i1 false)
   ret void
 }
 
 define i32 @var_args(i32 %i1, ...) #0 {
 ; CHECK-LABEL: var_args:

llvm/test/CodeGen/X86/memset-2.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=i386-apple-darwin9 -mcpu=yonah < %s | FileCheck %s
 
-define fastcc void @t1() nounwind {
+define fastcc void @t1(ptr nocapture %s) nounwind {
 ; CHECK-LABEL: t1:
 ; CHECK:       ## %bb.0: ## %entry
 ; CHECK-NEXT:    subl $16, %esp
 ; CHECK-NEXT:    pushl $188
 ; CHECK-NEXT:    pushl $0
-; CHECK-NEXT:    pushl $0
+; CHECK-NEXT:    pushl %ecx
 ; CHECK-NEXT:    calll _memset
 ; CHECK-NEXT:    addl $16, %esp
 ; CHECK-NEXT:    ud2
 entry:
-  call void @llvm.memset.p0.i32(ptr null, i8 0, i32 188, i1 false)
+  call void @llvm.memset.p0.i32(ptr %s, i8 0, i32 188, i1 false)
   unreachable
 }
 
-define fastcc void @t2(i8 signext %c) nounwind {
+define fastcc void @t2(ptr nocapture %s, i8 signext %c) nounwind {
 ; CHECK-LABEL: t2:
 ; CHECK:       ## %bb.0: ## %entry
-; CHECK-NEXT:    subl $12, %esp
-; CHECK-NEXT:    movl %ecx, {{[0-9]+}}(%esp)
-; CHECK-NEXT:    movl $76, {{[0-9]+}}(%esp)
+; CHECK-NEXT:    subl $16, %esp
+; CHECK-NEXT:    pushl $76
+; CHECK-NEXT:    pushl %edx
+; CHECK-NEXT:    pushl %ecx
 ; CHECK-NEXT:    calll _memset
+; CHECK-NEXT:    addl $16, %esp
 ; CHECK-NEXT:    ud2
 entry:
-  call void @llvm.memset.p0.i32(ptr undef, i8 %c, i32 76, i1 false)
+  call void @llvm.memset.p0.i32(ptr %s, i8 %c, i32 76, i1 false)
   unreachable
 }
 
 declare void @llvm.memset.p0.i32(ptr nocapture, i8, i32, i1) nounwind
 
 define void @t3(ptr nocapture %s, i8 %a) nounwind {
 ; CHECK-LABEL: t3:
 ; CHECK:       ## %bb.0: ## %entry

llvm/test/CodeGen/X86/memset64-on-x86-32.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=i386-unknown-unknown   -mattr=sse4.2 | FileCheck %s --check-prefix=FAST
 ; RUN: llc < %s -mtriple=i386-unknown-unknown   -mattr=ssse3   | FileCheck %s --check-prefix=SLOW_32
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=ssse3   | FileCheck %s --check-prefix=SLOW_64
 
-define void @bork() nounwind {
+define void @bork(ptr nocapture align 4 %dst) nounwind {
 ; FAST-LABEL: bork:
 ; FAST:       # %bb.0:
+; FAST-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; FAST-NEXT:    xorps %xmm0, %xmm0
-; FAST-NEXT:    movups %xmm0, 64
-; FAST-NEXT:    movups %xmm0, 48
-; FAST-NEXT:    movups %xmm0, 32
-; FAST-NEXT:    movups %xmm0, 16
-; FAST-NEXT:    movups %xmm0, 0
+; FAST-NEXT:    movups %xmm0, 64(%eax)
+; FAST-NEXT:    movups %xmm0, 48(%eax)
+; FAST-NEXT:    movups %xmm0, 32(%eax)
+; FAST-NEXT:    movups %xmm0, 16(%eax)
+; FAST-NEXT:    movups %xmm0, (%eax)
 ; FAST-NEXT:    retl
 ;
 ; SLOW_32-LABEL: bork:
 ; SLOW_32:       # %bb.0:
-; SLOW_32-NEXT:    movl $0, 4
-; SLOW_32-NEXT:    movl $0, 0
-; SLOW_32-NEXT:    movl $0, 12
-; SLOW_32-NEXT:    movl $0, 8
-; SLOW_32-NEXT:    movl $0, 20
-; SLOW_32-NEXT:    movl $0, 16
-; SLOW_32-NEXT:    movl $0, 28
-; SLOW_32-NEXT:    movl $0, 24
-; SLOW_32-NEXT:    movl $0, 36
-; SLOW_32-NEXT:    movl $0, 32
-; SLOW_32-NEXT:    movl $0, 44
-; SLOW_32-NEXT:    movl $0, 40
-; SLOW_32-NEXT:    movl $0, 52
-; SLOW_32-NEXT:    movl $0, 48
-; SLOW_32-NEXT:    movl $0, 60
-; SLOW_32-NEXT:    movl $0, 56
-; SLOW_32-NEXT:    movl $0, 68
-; SLOW_32-NEXT:    movl $0, 64
-; SLOW_32-NEXT:    movl $0, 76
-; SLOW_32-NEXT:    movl $0, 72
+; SLOW_32-NEXT:    movl {{[0-9]+}}(%esp), %eax
+; SLOW_32-NEXT:    movl $0, 4(%eax)
+; SLOW_32-NEXT:    movl $0, (%eax)
+; SLOW_32-NEXT:    movl $0, 12(%eax)
+; SLOW_32-NEXT:    movl $0, 8(%eax)
+; SLOW_32-NEXT:    movl $0, 20(%eax)
+; SLOW_32-NEXT:    movl $0, 16(%eax)
+; SLOW_32-NEXT:    movl $0, 28(%eax)
+; SLOW_32-NEXT:    movl $0, 24(%eax)
+; SLOW_32-NEXT:    movl $0, 36(%eax)
+; SLOW_32-NEXT:    movl $0, 32(%eax)
+; SLOW_32-NEXT:    movl $0, 44(%eax)
+; SLOW_32-NEXT:    movl $0, 40(%eax)
+; SLOW_32-NEXT:    movl $0, 52(%eax)
+; SLOW_32-NEXT:    movl $0, 48(%eax)
+; SLOW_32-NEXT:    movl $0, 60(%eax)
+; SLOW_32-NEXT:    movl $0, 56(%eax)
+; SLOW_32-NEXT:    movl $0, 68(%eax)
+; SLOW_32-NEXT:    movl $0, 64(%eax)
+; SLOW_32-NEXT:    movl $0, 76(%eax)
+; SLOW_32-NEXT:    movl $0, 72(%eax)
 ; SLOW_32-NEXT:    retl
 ;
 ; SLOW_64-LABEL: bork:
 ; SLOW_64:       # %bb.0:
-; SLOW_64-NEXT:    movq $0, 72
-; SLOW_64-NEXT:    movq $0, 64
-; SLOW_64-NEXT:    movq $0, 56
-; SLOW_64-NEXT:    movq $0, 48
-; SLOW_64-NEXT:    movq $0, 40
-; SLOW_64-NEXT:    movq $0, 32
-; SLOW_64-NEXT:    movq $0, 24
-; SLOW_64-NEXT:    movq $0, 16
-; SLOW_64-NEXT:    movq $0, 8
-; SLOW_64-NEXT:    movq $0, 0
+; SLOW_64-NEXT:    movq $0, 72(%rdi)
+; SLOW_64-NEXT:    movq $0, 64(%rdi)
+; SLOW_64-NEXT:    movq $0, 56(%rdi)
+; SLOW_64-NEXT:    movq $0, 48(%rdi)
+; SLOW_64-NEXT:    movq $0, 40(%rdi)
+; SLOW_64-NEXT:    movq $0, 32(%rdi)
+; SLOW_64-NEXT:    movq $0, 24(%rdi)
+; SLOW_64-NEXT:    movq $0, 16(%rdi)
+; SLOW_64-NEXT:    movq $0, 8(%rdi)
+; SLOW_64-NEXT:    movq $0, (%rdi)
 ; SLOW_64-NEXT:    retq
-  call void @llvm.memset.p0.i64(ptr align 4 null, i8 0, i64 80, i1 false)
+  call void @llvm.memset.p0.i64(ptr align 4 %dst, i8 0, i64 80, i1 false)
   ret void
 }
 
 declare void @llvm.memset.p0.i64(ptr nocapture, i8, i64, i1) nounwind