[llvm] r290844 - [InstCombine] use combineMetadataForCSE instead of copying it; NFCI

[Sanjay Patel via llvm-commits]

Ah, right. I hacked my local source with this (I'm guessing this isn't
safe...) to see the subsequent problem:

Index: lib/Analysis/Loads.cpp
===================================================================
--- lib/Analysis/Loads.cpp    (revision 290827)
+++ lib/Analysis/Loads.cpp    (working copy)
@@ -382,8 +382,8 @@
           return nullptr;

         if (IsLoadCSE)
-          *IsLoadCSE = false;
-        return SI->getOperand(0);
+          *IsLoadCSE = isa<LoadInst>(SI->getValueOperand());
+        return SI->getValueOperand();
       }

       // If both StrippedPtr and StorePtr reach all the way to an alloca or


On Tue, Jan 3, 2017 at 9:11 AM, Davide Italiano <davide at freebsd.org> wrote:

> On Tue, Jan 3, 2017 at 7:58 AM, Sanjay Patel <spatel at rotateright.com>
> wrote:
> > That's what the code comment would have us believe, but... :)
> >
> > define i8* @isa_impl_wrap(i8** %x) {
> >   %t2 = alloca i8*
> >   %t4 = load i8*, i8** %x, !nonnull !0
> >   store i8* %t4, i8** %t2
> >   %t6 = load i8*, i8** %t2
> >   ret i8* %t6
> > }
> > !0 = !{}
> >
> > $ ./opt -instcombine cse_nonnull.ll -S
> > ...
> > define i8* @isa_impl_wrap(i8** %x) {
> >   %t4 = load i8*, i8** %x, align 8, !nonnull !0 <--- nonnull preserved
> when
> > earlier instruction has it
> >   ret i8* %t4
> > }
> >
>
> So, it's not that `combineMetadataForCSE` not behaving ideally. When
> instcombine runs on your intermediate you don't call it at all.
>
> Debug:
> IC: Visiting:   %t6 = load i8*, i8** %t2
> [...]
>
> (gdb) p LI.dump()
>   %t6 = load i8*, i8** %t2, align 8
> $2 = void
> (gdb) p IsLoadCSE
> $3 = false
> (gdb) list
> 853       if (Value *AvailableVal = FindAvailableLoadedValue(
> 854               &LI, LI.getParent(), BBI, DefMaxInstsToScan, AA,
> &IsLoadCSE)) {
> 855         if (IsLoadCSE)
> 856           combineMetadataForCSE(cast<LoadInst>(AvailableVal), &LI);
> 857
> 858         return replaceInstUsesWith(
> 859             LI, Builder->CreateBitOrPointerCast(AvailableVal,
> LI.getType(),
> 860                                                 LI.getName() +
> ".cast"));
> 861       }
>
> So we call RAUW without combining metadata, (unless I'm missing something).
>
> --
> Davide
>
> "There are no solved problems; there are only problems that are more
> or less solved" -- Henri Poincare
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170103/0ddecee7/attachment.html>