[llvm] 7abefc4 - [instcombine] Fold away memset/memmove from otherwise unused alloca

Wed Mar 23 10:09:09 PDT 2022

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/65efb0ec/attachment.html>