<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 3, 2017 at 4:29 AM, Davide Italiano <span dir="ltr"><<a href="mailto:davide@freebsd.org" target="_blank">davide@freebsd.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail-HOEnZb"><div class="gmail-h5">On Mon, Jan 2, 2017 at 3:25 PM, Sanjay Patel via llvm-commits<br>
<<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
> Author: spatel<br>
> Date: Mon Jan  2 17:25:28 2017<br>
> New Revision: 290844<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=290844&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=290844&view=rev</a><br>
> Log:<br>
> [InstCombine] use combineMetadataForCSE instead of copying it; NFCI<br>
><br>
> Modified:<br>
>     llvm/trunk/lib/Transforms/<wbr>InstCombine/<wbr>InstCombineLoadStoreAlloca.cpp<br>
><br>
> Modified: llvm/trunk/lib/Transforms/<wbr>InstCombine/<wbr>InstCombineLoadStoreAlloca.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp?rev=290844&r1=290843&r2=290844&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/<wbr>Transforms/InstCombine/<wbr>InstCombineLoadStoreAlloca.<wbr>cpp?rev=290844&r1=290843&r2=<wbr>290844&view=diff</a><br>
> ==============================<wbr>==============================<wbr>==================<br>
> --- llvm/trunk/lib/Transforms/<wbr>InstCombine/<wbr>InstCombineLoadStoreAlloca.cpp (original)<br>
> +++ llvm/trunk/lib/Transforms/<wbr>InstCombine/<wbr>InstCombineLoadStoreAlloca.cpp Mon Jan  2 17:25:28 2017<br>
> @@ -850,20 +850,10 @@ Instruction *InstCombiner::visitLoadInst<br>
>    // separated by a few arithmetic operations.<br>
>    BasicBlock::iterator BBI(LI);<br>
>    bool IsLoadCSE = false;<br>
> -  if (Value *AvailableVal =<br>
> -      FindAvailableLoadedValue(&LI, LI.getParent(), BBI,<br>
> -                               DefMaxInstsToScan, AA, &IsLoadCSE)) {<br>
> -    if (IsLoadCSE) {<br>
> -      LoadInst *NLI = cast<LoadInst>(AvailableVal);<br>
> -      unsigned KnownIDs[] = {<br>
> -          LLVMContext::MD_tbaa,            LLVMContext::MD_alias_scope,<br>
> -          LLVMContext::MD_noalias,         LLVMContext::MD_range,<br>
> -          LLVMContext::MD_invariant_<wbr>load,  LLVMContext::MD_nonnull,<br>
> -          LLVMContext::MD_invariant_<wbr>group, LLVMContext::MD_align,<br>
> -          LLVMContext::MD_<wbr>dereferenceable,<br>
> -          LLVMContext::MD_<wbr>dereferenceable_or_null};<br>
> -      combineMetadata(NLI, &LI, KnownIDs);<br>
> -    };<br>
> +  if (Value *AvailableVal = FindAvailableLoadedValue(<br>
> +          &LI, LI.getParent(), BBI, DefMaxInstsToScan, AA, &IsLoadCSE)) {<br>
> +    if (IsLoadCSE)<br>
> +      combineMetadataForCSE(cast<<wbr>LoadInst>(AvailableVal), &LI);<br>
><br>
<br>
</div></div>We have still a lot of places where we have fundamentally the same'ish<br>
version of this code in the tree<br>
(GVN/GVNHoist/SimplifyCFG/<wbr>BBVectorize), is it possible to convert to<br>
`combineMetadataForCSE` (or there's some caveat here I'm missing)?<br>
<span class="gmail-HOEnZb"></span></blockquote><div><br><br></div><div>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:<br><br>define i8* @isa_impl_wrap(i8** %x) {<br>  %t2 = alloca i8*<br>  %t4 = load i8*, i8** %x<br>  store i8* %t4, i8** %t2<br>  %t6 = load i8*, i8** %t2, !nonnull !0   <--- we should preserve that to optimize dyn_cast<br>  ret i8* %t6<br>}<br>!0 = !{}<br><br>$ ./opt -instcombine cse_nonnull.ll -S<br>...<br>define i8* @isa_impl_wrap(i8** %x) {<br>  %t4 = load i8*, i8** %x, align 8   <--- but it's gone now <br>  ret i8* %t4<br>}<br><br></div></div></div></div>