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

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 3 07:21:46 PST 2017


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

> On Mon, Jan 2, 2017 at 3:25 PM, Sanjay Patel via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> > Author: spatel
> > Date: Mon Jan  2 17:25:28 2017
> > New Revision: 290844
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=290844&view=rev
> > Log:
> > [InstCombine] use combineMetadataForCSE instead of copying it; NFCI
> >
> > Modified:
> >     llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> >
> > Modified: llvm/trunk/lib/Transforms/InstCombine/
> InstCombineLoadStoreAlloca.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/
> Transforms/InstCombine/InstCombineLoadStoreAlloca.
> cpp?rev=290844&r1=290843&r2=290844&view=diff
> > ============================================================
> ==================
> > --- llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> (original)
> > +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> Mon Jan  2 17:25:28 2017
> > @@ -850,20 +850,10 @@ Instruction *InstCombiner::visitLoadInst
> >    // separated by a few arithmetic operations.
> >    BasicBlock::iterator BBI(LI);
> >    bool IsLoadCSE = false;
> > -  if (Value *AvailableVal =
> > -      FindAvailableLoadedValue(&LI, LI.getParent(), BBI,
> > -                               DefMaxInstsToScan, AA, &IsLoadCSE)) {
> > -    if (IsLoadCSE) {
> > -      LoadInst *NLI = cast<LoadInst>(AvailableVal);
> > -      unsigned KnownIDs[] = {
> > -          LLVMContext::MD_tbaa,            LLVMContext::MD_alias_scope,
> > -          LLVMContext::MD_noalias,         LLVMContext::MD_range,
> > -          LLVMContext::MD_invariant_load,  LLVMContext::MD_nonnull,
> > -          LLVMContext::MD_invariant_group, LLVMContext::MD_align,
> > -          LLVMContext::MD_dereferenceable,
> > -          LLVMContext::MD_dereferenceable_or_null};
> > -      combineMetadata(NLI, &LI, KnownIDs);
> > -    };
> > +  if (Value *AvailableVal = FindAvailableLoadedValue(
> > +          &LI, LI.getParent(), BBI, DefMaxInstsToScan, AA, &IsLoadCSE))
> {
> > +    if (IsLoadCSE)
> > +      combineMetadataForCSE(cast<LoadInst>(AvailableVal), &LI);
> >
>
> We have still a lot of places where we have fundamentally the same'ish
> version of this code in the tree
> (GVN/GVNHoist/SimplifyCFG/BBVectorize), is it possible to convert to
> `combineMetadataForCSE` (or there's some caveat here I'm missing)?
>


Add VectorUtils.cpp -> llvm::propagateMetadata() to the list; that's used
by the Loop and SLP vectorizers. I've never looked at this closely (I'm
here in the context of PR28430), so I'm not sure what, if any, differences
these callers expect. It's also seems like combineMetadataForCSE is not
behaving ideally. For example, I got here because:

define i8* @isa_impl_wrap(i8** %x) {
  %t2 = alloca i8*
  %t4 = load i8*, i8** %x
  store i8* %t4, i8** %t2
  %t6 = load i8*, i8** %t2, !nonnull !0   <--- we should preserve that to
optimize dyn_cast
  ret i8* %t6
}
!0 = !{}

$ ./opt -instcombine cse_nonnull.ll -S
...
define i8* @isa_impl_wrap(i8** %x) {
  %t4 = load i8*, i8** %x, align 8   <--- but it's gone now
  ret i8* %t4
}
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170103/d3de260d/attachment.html>


More information about the llvm-commits mailing list