From 20c9207be3d935c3759220cfee173022d1bff2db Mon Sep 17 00:00:00 2001 From: Daniel Neilson Date: Thu, 22 Feb 2018 18:55:59 +0000 Subject: [PATCH] [AlignmentFromAssumptions] Set source and dest alignments of memory intrinsiscs separately Summary: This change is part of step five in the series of changes to remove alignment argument from memcpy/memmove/memset in favour of alignment attributes. In particular, this changes the AlignmentFromAssumptions pass to cease using the old getAlignment()/setAlignment API of MemoryIntrinsic in favour of getting/setting source & dest specific alignments through the new API. This allows us to simplify some of the code in this pass and also be more aggressive about setting the source and destination alignments separately. Steps: Step 1) Remove alignment parameter and create alignment parameter attributes for memcpy/memmove/memset. ( rL322965, rC322964, rL322963 ) Step 2) Expand the IRBuilder API to allow creation of memcpy/memmove with differing source and dest alignments. ( rL323597 ) Step 3) Update Clang to use the new IRBuilder API. ( rC323617 ) Step 4) Update Polly to use the new IRBuilder API. ( rL323618 ) Step 5) Update LLVM passes that create memcpy/memmove calls to use the new IRBuilder API, and those that use use MemIntrinsicInst::[get|set]Alignment() to use [get|set]DestAlignment() and [get|set]SourceAlignment() instead. ( rL323886, rL323891, rL324148, rL324273, rL324278, rL324384, rL324395, rL324402, rL324626, rL324642, rL324653, rL324654, rL324773, rL324774, rL324781, rL324784, rL324955, rL324960 ) Step 6) Remove the single-alignment IRBuilder API for memcpy/memmove, and the MemIntrinsicInst::[get|set]Alignment() methods. Reference http://lists.llvm.org/pipermail/llvm-dev/2015-August/089384.html http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20151109/312083.html Reviewers: hfinkel, bollu, reames Reviewed By: reames Subscribers: reames, llvm-commits Differential Revision: https://reviews.llvm.org/D43081 llvm-svn: 325816 --- .../Scalar/AlignmentFromAssumptions.h | 6 --- .../Scalar/AlignmentFromAssumptions.cpp | 54 ++++--------------- .../AlignmentFromAssumptions/simple.ll | 2 +- .../AlignmentFromAssumptions/simple32.ll | 2 +- 4 files changed, 13 insertions(+), 51 deletions(-) diff --git a/llvm/include/llvm/Transforms/Scalar/AlignmentFromAssumptions.h b/llvm/include/llvm/Transforms/Scalar/AlignmentFromAssumptions.h index f75dc4dc331d..61975036e9ff 100644 --- a/llvm/include/llvm/Transforms/Scalar/AlignmentFromAssumptions.h +++ b/llvm/include/llvm/Transforms/Scalar/AlignmentFromAssumptions.h @@ -33,12 +33,6 @@ struct AlignmentFromAssumptionsPass bool runImpl(Function &F, AssumptionCache &AC, ScalarEvolution *SE_, DominatorTree *DT_); - // For memory transfers, we need a common alignment for both the source and - // destination. If we have a new alignment for only one operand of a transfer - // instruction, save it in these maps. If we reach the other operand through - // another assumption later, then we may change the alignment at that point. - DenseMap NewDestAlignments, NewSrcAlignments; - ScalarEvolution *SE = nullptr; DominatorTree *DT = nullptr; diff --git a/llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp b/llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp index 6c871bb9e7eb..b84528271a70 100644 --- a/llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp +++ b/llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp @@ -339,53 +339,24 @@ bool AlignmentFromAssumptionsPass::processAssumption(CallInst *ACall) { unsigned NewDestAlignment = getNewAlignment(AASCEV, AlignSCEV, OffSCEV, MI->getDest(), SE); - // For memory transfers, we need a common alignment for both the - // source and destination. If we have a new alignment for this - // instruction, but only for one operand, save it. If we reach the - // other operand through another assumption later, then we may - // change the alignment at that point. + DEBUG(dbgs() << "\tmem inst: " << NewDestAlignment << "\n";); + if (NewDestAlignment > MI->getDestAlignment()) { + MI->setDestAlignment(NewDestAlignment); + ++NumMemIntAlignChanged; + } + + // For memory transfers, there is also a source alignment that + // can be set. if (MemTransferInst *MTI = dyn_cast(MI)) { unsigned NewSrcAlignment = getNewAlignment(AASCEV, AlignSCEV, OffSCEV, MTI->getSource(), SE); - DenseMap::iterator DI = - NewDestAlignments.find(MTI); - unsigned AltDestAlignment = (DI == NewDestAlignments.end()) ? - 0 : DI->second; + DEBUG(dbgs() << "\tmem trans: " << NewSrcAlignment << "\n";); - DenseMap::iterator SI = - NewSrcAlignments.find(MTI); - unsigned AltSrcAlignment = (SI == NewSrcAlignments.end()) ? - 0 : SI->second; - - DEBUG(dbgs() << "\tmem trans: " << NewDestAlignment << " " << - AltDestAlignment << " " << NewSrcAlignment << - " " << AltSrcAlignment << "\n"); - - // Of these four alignments, pick the largest possible... - unsigned NewAlignment = 0; - if (NewDestAlignment <= std::max(NewSrcAlignment, AltSrcAlignment)) - NewAlignment = std::max(NewAlignment, NewDestAlignment); - if (AltDestAlignment <= std::max(NewSrcAlignment, AltSrcAlignment)) - NewAlignment = std::max(NewAlignment, AltDestAlignment); - if (NewSrcAlignment <= std::max(NewDestAlignment, AltDestAlignment)) - NewAlignment = std::max(NewAlignment, NewSrcAlignment); - if (AltSrcAlignment <= std::max(NewDestAlignment, AltDestAlignment)) - NewAlignment = std::max(NewAlignment, AltSrcAlignment); - - if (NewAlignment > MI->getAlignment()) { - MI->setAlignment(NewAlignment); + if (NewSrcAlignment > MTI->getSourceAlignment()) { + MTI->setSourceAlignment(NewSrcAlignment); ++NumMemIntAlignChanged; } - - NewDestAlignments.insert(std::make_pair(MTI, NewDestAlignment)); - NewSrcAlignments.insert(std::make_pair(MTI, NewSrcAlignment)); - } else if (NewDestAlignment > MI->getAlignment()) { - assert((!isa(MI) || isa(MI)) && - "Unknown memory intrinsic"); - - MI->setAlignment(NewDestAlignment); - ++NumMemIntAlignChanged; } } @@ -419,9 +390,6 @@ bool AlignmentFromAssumptionsPass::runImpl(Function &F, AssumptionCache &AC, SE = SE_; DT = DT_; - NewDestAlignments.clear(); - NewSrcAlignments.clear(); - bool Changed = false; for (auto &AssumeVH : AC.assumptions()) if (AssumeVH) diff --git a/llvm/test/Transforms/AlignmentFromAssumptions/simple.ll b/llvm/test/Transforms/AlignmentFromAssumptions/simple.ll index 8bbf1c668c99..6ee08b81e275 100644 --- a/llvm/test/Transforms/AlignmentFromAssumptions/simple.ll +++ b/llvm/test/Transforms/AlignmentFromAssumptions/simple.ll @@ -205,7 +205,7 @@ entry: ret i32 undef ; CHECK-LABEL: @moo2 -; CHECK: @llvm.memcpy.p0i8.p0i8.i64(i8* align 32 %0, i8* align 32 %1, i64 64, i1 false) +; CHECK: @llvm.memcpy.p0i8.p0i8.i64(i8* align 32 %0, i8* align 128 %1, i64 64, i1 false) ; CHECK: ret i32 undef } diff --git a/llvm/test/Transforms/AlignmentFromAssumptions/simple32.ll b/llvm/test/Transforms/AlignmentFromAssumptions/simple32.ll index 379a184fd7dd..5aabe9518415 100644 --- a/llvm/test/Transforms/AlignmentFromAssumptions/simple32.ll +++ b/llvm/test/Transforms/AlignmentFromAssumptions/simple32.ll @@ -205,7 +205,7 @@ entry: ret i32 undef ; CHECK-LABEL: @moo2 -; CHECK: @llvm.memcpy.p0i8.p0i8.i64(i8* align 32 %0, i8* align 32 %1, i64 64, i1 false) +; CHECK: @llvm.memcpy.p0i8.p0i8.i64(i8* align 32 %0, i8* align 128 %1, i64 64, i1 false) ; CHECK: ret i32 undef }