[llvm] 7abefc4 - [instcombine] Fold away memset/memmove from otherwise unused alloca
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 23 11:05:39 PDT 2022
Well, given your view as the bot owner, I'm going to just leave things
as is. I don't like the notion of leaving a bot red, but I don't see
any way for me to help figuring out what's going on here. Let me know
if anything leads you to believe this is a problem with the change after
all.
Philip
On 3/23/22 10:09, Reid Kleckner wrote:
>
> My guess is that this failure is unrelated to your change. I blame the
> MSVC linker, which is producing these errors. I forced a clean build,
> but it looks like it didn't do anything.
>
> I think we should move to using lld-link across the Windows ASan test
> suite. It doesn't try to do incremental linking, so it won't have
> sticky file format corruption errors like this. This removes some
> degree of integration testing, so we might want to bring back some
> explicit integration testing.
>
> Also, IIRC Matthew left Microsoft some time ago, or changed teams there.
>
> On Wed, Mar 23, 2022 at 7:30 AM Philip Reames
> <listmail at philipreames.com> wrote:
>
> Reid, Matthew,
>
> You two are the bot owner and the last person to touch this test
> respectively. Any chance I could ask for some help here?
>
> I'm fairly sure the bot failure mentioned below is a real failure,
> but
> a) no other bot has failed, and b) I can't translate how removing a
> memcpy could lead to "LINK : fatal error LNK1318: Unexpected PDB
> error;
> FORMAT (11) ''". Any idea what might be going on with this test? Or
> ideas on how to reproduce it which don't require a windows build?
>
> I *suspect* this is a case where the test needs adjusted to prevent
> something being optimized away - that's the typical asan test pattern
> for new memcpy opts - but I might be wrong about that.
>
> Philip
>
> On 3/22/22 14:46, Philip Reames via llvm-commits wrote:
> > I've got one buildbot which might be failing on this:
> > https://lab.llvm.org/buildbot#builders/127/builds/26790
> >
> > However, I can't make heads or tails out the failure message and
> the
> > test isn't obvious unstable. Waiting to see if any Linux bots fail
> > with something I can understand.
> >
> > Philip
> >
> > On 3/22/22 13:49, Philip Reames via llvm-commits wrote:
> >> Author: Philip Reames
> >> Date: 2022-03-22T13:48:48-07:00
> >> New Revision: 7abefc42220b74551b433083ece33be31e48700f
> >>
> >> URL:
> >>
> https://github.com/llvm/llvm-project/commit/7abefc42220b74551b433083ece33be31e48700f
> >> DIFF:
> >>
> https://github.com/llvm/llvm-project/commit/7abefc42220b74551b433083ece33be31e48700f.diff
> >>
> >> LOG: [instcombine] Fold away memset/memmove from otherwise
> unused alloca
> >>
> >> The motivation for this is that while both memcpyopt and dse will
> >> catch this case, both are limited by MSSA's walk back threshold
> when
> >> finding clobbers. As such, if you have a memcpy of an
> otherwise dead
> >> alloca placed towards the end of a long basic block with lots of
> >> other memory instructions, it would be missed. This is a bit
> >> undesirable for such an "obviously" useless bit of code.
> >>
> >> As noted in comments, we should probably generalize instcombine's
> >> escape analysis peephole (see visitAllocInst) to allow read xor
> >> write. Doing that would subsume this code in a more general
> way, but
> >> is also a more involved change. For the moment, I went with the
> >> easiest fix.
> >>
> >> Added:
> >>
> >> Modified:
> >> llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
> >> llvm/test/Transforms/Inline/byval-tail-call.ll
> >> llvm/test/Transforms/InstCombine/memcpy_alloca.ll
> >>
> >> Removed:
> >>
> >>
> >>
> ################################################################################
>
> >>
> >> diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
> >> b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
> >> index 9327dda3924dc..b3edec04fbcef 100644
> >> --- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
> >> +++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
> >> @@ -104,6 +104,19 @@ static Type *getPromotedType(Type *Ty) {
> >> return Ty;
> >> }
> >> +/// Recognize a memcpy/memmove from a trivially otherwise
> unused
> >> alloca.
> >> +/// TODO: This should probably be integrated with
> visitAllocSites,
> >> but that
> >> +/// requires a deeper change to allow either unread or unwritten
> >> objects.
> >> +static bool hasUndefSource(AnyMemTransferInst *MI) {
> >> + auto *Src = MI->getRawSource();
> >> + while (isa<GetElementPtrInst>(Src) || isa<BitCastInst>(Src)) {
> >> + if (!Src->hasOneUse())
> >> + return false;
> >> + Src = cast<Instruction>(Src)->getOperand(0);
> >> + }
> >> + return isa<AllocaInst>(Src) && Src->hasOneUse();
> >> +}
> >> +
> >> Instruction
> >> *InstCombinerImpl::SimplifyAnyMemTransfer(AnyMemTransferInst *MI) {
> >> Align DstAlign = getKnownAlignment(MI->getRawDest(), DL,
> MI, &AC,
> >> &DT);
> >> MaybeAlign CopyDstAlign = MI->getDestAlign();
> >> @@ -128,6 +141,14 @@ Instruction
> >> *InstCombinerImpl::SimplifyAnyMemTransfer(AnyMemTransferInst *MI) {
> >> return MI;
> >> }
> >> + // If the source is provably undef, the memcpy/memmove
> doesn't
> >> do anything
> >> + // (unless the transfer is volatile).
> >> + if (hasUndefSource(MI) && !MI->isVolatile()) {
> >> + // Set the size of the copy to 0, it will be deleted on
> the next
> >> iteration.
> >> +
> MI->setLength(Constant::getNullValue(MI->getLength()->getType()));
> >> + return MI;
> >> + }
> >> +
> >> // If MemCpyInst length is 1/2/4/8 bytes then replace
> memcpy with
> >> // load/store.
> >> ConstantInt *MemOpLength =
> dyn_cast<ConstantInt>(MI->getLength());
> >>
> >> diff --git a/llvm/test/Transforms/Inline/byval-tail-call.ll
> >> b/llvm/test/Transforms/Inline/byval-tail-call.ll
> >> index a820bcb427dcf..19be5e5827f04 100644
> >> --- a/llvm/test/Transforms/Inline/byval-tail-call.ll
> >> +++ b/llvm/test/Transforms/Inline/byval-tail-call.ll
> >> @@ -92,14 +92,11 @@ define void @foobar(i32* %x) {
> >> define void @barfoo() {
> >> ; CHECK-LABEL: @barfoo(
> >> ; CHECK-NEXT: [[X1:%.*]] = alloca i32, align 4
> >> -; CHECK-NEXT: [[X:%.*]] = alloca i32, align 4
> >> ; CHECK-NEXT: [[TMP1:%.*]] = bitcast i32* [[X1]] to i8*
> >> ; CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 4, i8*
> >> nonnull [[TMP1]])
> >> -; CHECK-NEXT: [[TMP2:%.*]] = load i32, i32* [[X]], align 4
> >> -; CHECK-NEXT: store i32 [[TMP2]], i32* [[X1]], align 4
> >> ; CHECK-NEXT: tail call void @ext2(i32* nonnull byval(i32)
> [[X1]])
> >> -; CHECK-NEXT: [[TMP3:%.*]] = bitcast i32* [[X1]] to i8*
> >> -; CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 4, i8*
> >> nonnull [[TMP3]])
> >> +; CHECK-NEXT: [[TMP2:%.*]] = bitcast i32* [[X1]] to i8*
> >> +; CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 4, i8*
> >> nonnull [[TMP2]])
> >> ; CHECK-NEXT: ret void
> >> ;
> >> %x = alloca i32
> >>
> >> diff --git a/llvm/test/Transforms/InstCombine/memcpy_alloca.ll
> >> b/llvm/test/Transforms/InstCombine/memcpy_alloca.ll
> >> index b7288d9f07476..fabf920c6e68d 100644
> >> --- a/llvm/test/Transforms/InstCombine/memcpy_alloca.ll
> >> +++ b/llvm/test/Transforms/InstCombine/memcpy_alloca.ll
> >> @@ -4,9 +4,6 @@
> >> ; Memcpy is copying known-undef, and is thus removable
> >> define void @test(i8* %dest) {
> >> ; CHECK-LABEL: @test(
> >> -; CHECK-NEXT: [[A:%.*]] = alloca [7 x i8], align 1
> >> -; CHECK-NEXT: [[SRC:%.*]] = getelementptr inbounds [7 x
> i8], [7 x
> >> i8]* [[A]], i64 0, i64 0
> >> -; CHECK-NEXT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* noundef
> >> nonnull align 1 dereferenceable(7) [[DEST:%.*]], i8* noundef
> nonnull
> >> align 1 dereferenceable(7) [[SRC]], i64 7, i1 false)
> >> ; CHECK-NEXT: ret void
> >> ;
> >> %a = alloca [7 x i8]
> >> @@ -47,9 +44,6 @@ define void @test3(i8* %dest) {
> >> define void @test4(i8* %dest) {
> >> ; CHECK-LABEL: @test4(
> >> -; CHECK-NEXT: [[A1:%.*]] = alloca [7 x i8], align 1
> >> -; CHECK-NEXT: [[A1_SUB:%.*]] = getelementptr inbounds [7 x
> i8],
> >> [7 x i8]* [[A1]], i64 0, i64 0
> >> -; CHECK-NEXT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* noundef
> >> nonnull align 1 dereferenceable(7) [[DEST:%.*]], i8* noundef
> nonnull
> >> align 1 dereferenceable(7) [[A1_SUB]], i64 7, i1 false)
> >> ; CHECK-NEXT: ret void
> >> ;
> >> %a = alloca [7 x i8]
> >> @@ -60,9 +54,6 @@ define void @test4(i8* %dest) {
> >> define void @test5(i8* %dest) {
> >> ; CHECK-LABEL: @test5(
> >> -; CHECK-NEXT: [[A:%.*]] = alloca [7 x i8], align 1
> >> -; CHECK-NEXT: [[P2:%.*]] = getelementptr inbounds [7 x i8],
> [7 x
> >> i8]* [[A]], i64 0, i64 4
> >> -; CHECK-NEXT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* noundef
> >> nonnull align 1 dereferenceable(3) [[DEST:%.*]], i8* noundef
> nonnull
> >> align 1 dereferenceable(3) [[P2]], i64 3, i1 false)
> >> ; CHECK-NEXT: ret void
> >> ;
> >> %a = alloca [7 x i8]
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at lists.llvm.org
> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220323/f69ef444/attachment.html>
More information about the llvm-commits
mailing list