Commit ad9ae6ee authored by Juneyoung Lee's avatar Juneyoung Lee
Browse files

MemCpyOpt cannot use ABI alignment even if it was not given 

Summary: This patch fixes https://bugs.llvm.org/show_bug.cgi?id=44388 which incorrectly assigns an ABI alignment to memset when there was no explicit alignment given.

Reviewers: gchatelet, lenary, nikic

Reviewed By: nikic

Subscribers: hiraditya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D74083
parent 2697e8bc

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

+21 −25
Original line number Diff line number Diff line
@@ -144,6 +144,24 @@ bool MemsetRange::isProfitableToUseMemset(const DataLayout &DL) const { 
  return TheStores.size() > NumPointerStores+NumByteStores;

}





static Align findStoreAlignment(const DataLayout &DL, const StoreInst *SI) {

  return DL.getValueOrABITypeAlignment(MaybeAlign(SI->getAlignment()),

                                       SI->getOperand(0)->getType());

}



static Align findLoadAlignment(const DataLayout &DL, const LoadInst *LI) {

  return DL.getValueOrABITypeAlignment(MaybeAlign(LI->getAlignment()),

                                       LI->getType());

}



static Align findCommonAlignment(const DataLayout &DL, const StoreInst *SI,

                                 const LoadInst *LI) {

  Align StoreAlign = findStoreAlignment(DL, SI);

  Align LoadAlign = findLoadAlignment(DL, LI);

  return commonAlignment(StoreAlign, LoadAlign);

}



namespace {



class MemsetRanges {
@@ -173,8 +191,8 @@ public: 
  void addStore(int64_t OffsetFromFirst, StoreInst *SI) {

    int64_t StoreSize = DL.getTypeStoreSize(SI->getOperand(0)->getType());



    addRange(OffsetFromFirst, StoreSize,

             SI->getPointerOperand(), SI->getAlignment(), SI);

    addRange(OffsetFromFirst, StoreSize, SI->getPointerOperand(),

             findStoreAlignment(DL, SI).value(), SI);

  }



  void addMemSet(int64_t OffsetFromFirst, MemSetInst *MSI) {
@@ -387,13 +405,8 @@ Instruction *MemCpyOptPass::tryMergingIntoMemset(Instruction *StartInst, 
    // Get the starting pointer of the block.

    StartPtr = Range.StartPtr;



    // Determine alignment

    const Align Alignment = DL.getValueOrABITypeAlignment(

        MaybeAlign(Range.Alignment),

        cast<PointerType>(StartPtr->getType())->getElementType());



    AMemSet = Builder.CreateMemSet(StartPtr, ByteVal, Range.End - Range.Start,

                                   Alignment);

                                   MaybeAlign(Range.Alignment));

    LLVM_DEBUG(dbgs() << "Replace stores:\n"; for (Instruction *SI

                                                   : Range.TheStores) dbgs()

                                              << *SI << '\n';
@@ -413,23 +426,6 @@ Instruction *MemCpyOptPass::tryMergingIntoMemset(Instruction *StartInst, 
  return AMemSet;

}



static Align findStoreAlignment(const DataLayout &DL, const StoreInst *SI) {

  return DL.getValueOrABITypeAlignment(MaybeAlign(SI->getAlignment()),

                                       SI->getOperand(0)->getType());

}



static Align findLoadAlignment(const DataLayout &DL, const LoadInst *LI) {

  return DL.getValueOrABITypeAlignment(MaybeAlign(LI->getAlignment()),

                                       LI->getType());

}



static Align findCommonAlignment(const DataLayout &DL, const StoreInst *SI,

                                 const LoadInst *LI) {

  Align StoreAlign = findStoreAlignment(DL, SI);

  Align LoadAlign = findLoadAlignment(DL, LI);

  return commonAlignment(StoreAlign, LoadAlign);

}



// This method try to lift a store instruction before position P.

// It will lift the store and its argument + that anything that

// may alias with these.

llvm/test/Transforms/MemCpyOpt/form-memset.ll

+15 −2
Original line number Diff line number Diff line
@@ -217,7 +217,7 @@ entry: 
  tail call void @llvm.memset.p0i8.i64(i8* %1, i8 0, i64 12, i1 false)

  ret void

; CHECK-LABEL: @test6(

; CHECK: call void @llvm.memset.p0i8.i64(i8* align 4 %2, i8 0, i64 24, i1 false)

; CHECK: call void @llvm.memset.p0i8.i64(i8* %2, i8 0, i64 24, i1 false)

}



; More aggressive heuristic
@@ -280,7 +280,7 @@ define void @test10(i8* nocapture %P) nounwind { 
  ret void

; CHECK-LABEL: @test10(

; CHECK-NOT: memset

; CHECK: call void @llvm.memset.p0i8.i64(i8* align 1 %P, i8 0, i64 42, i1 false)

; CHECK: call void @llvm.memset.p0i8.i64(i8* %P, i8 0, i64 42, i1 false)

; CHECK-NOT: memset

; CHECK: ret void

}
@@ -299,3 +299,16 @@ entry: 
; CHECK-NOT: store

; CHECK: call void @llvm.memset.p0i8.i64(i8* align 4 %1, i8 1, i64 23, i1 false)

}



; Alignment should be preserved when there is a store with default align

define void @test12(i32* nocapture %P) nounwind ssp {

entry:

  store i32 0, i32* %P

  %add.ptr = getelementptr inbounds i32, i32* %P, i64 1

  %0 = bitcast i32* %add.ptr to i8*

  tail call void @llvm.memset.p0i8.i64(i8* %0, i8 0, i64 11, i1 false)

  ret void

; CHECK-LABEL: @test12(

; CHECK-NOT: store

; CHECK: call void @llvm.memset.p0i8.i64(i8* align 4 %1, i8 0, i64 15, i1 false)

}