[PATCH] D40480: MemorySSA backed Dead Store Elimination.

[David Green via llvm-commits]

OK. Good stuff. Thanks for the info. Compile times I can do. The results don't look awful, so long as you squint a little :) They are not too bad, and hopefully we can make them a little better from here.

I'll try and clean up what I have right now and report on these compile time numbers. Watch this space.

Cheers for the info,
Dave


________________________________
From: Daniel Berlin <dberlin at dberlin.org>
Sent: 05 December 2017 04:30:54
To: David Green
Cc: reviews+D40480+public+ec7e4dc10b50dfd8 at reviews.llvm.org; Javed Absar; Piotr Padlewski; George Burgess IV; Friedman, Eli; Davide Italiano; llvm-commits at xorshift.org; llvm-commits
Subject: Re: [PATCH] D40480: MemorySSA backed Dead Store Elimination.

And note, obviously, you should do speed testing to see how slow it is right now.

I'm happy to approve this DSE if it's better, for sure.

But every time i see random limits like you have, i get very very worried.
The cases where we should not be able to perform basic memory optimizations like DSE should be incredibly large. I'm aware we currently limit it through memdep and other things.  I'm not even saying it's your problem. But it is worrying to me ;)



On Mon, Dec 4, 2017 at 11:09 AM, Daniel Berlin <dberlin at dberlin.org<mailto:dberlin at dberlin.org>> wrote:

Nope, you are right, part of this is giving up in some cases for removing a factor of O(uses) from most checks. But there is more to extract from what it is telling you that *isn't* doing that.


Remember that what it's really telling you is that "any def in between the use and the def it's linked to is no-alias of the use".

So, given, for example:

define i32 @test(i32* %P, i32 %y) {
; 1 = MemoryDef(liveOnEntry)
  store i32 1, i32* %P
  %Py = getelementptr i32, i32* %P, i32 %y
; 2 = MemoryDef(1)
  store i32 2, i32* %Py
; 3 = MemoryDef(2)
  store i32 1, i32* %P
  %P1 = getelementptr i32, i32* %P, i32 1
; MemoryUse(2)
  %l = load i32, i32* %P1
  ret i32 %l
}


This proves that MemoryUse(2) cannot affect the store at 1.  If it could, it would have been MemoryUse(3) instead.
MemorySSA already figured this out for you, and currently, you will rediscover it.  In fact, it's better than that, because in just about all cases outside of the pair metadata i referred to earlier, seeing this proves it "for all time" (IE you don't have to ever forget this information)

IE your statement " If SI->Current is NoAlias we can't safely treat SI->Use as noalias though."

This is true in general, but false if a store with the same loc is in between Si->Use and SI->getDefiningAccess()

You can use OrderedInstructions or something to see if that's the case.

So basically all i'm asking is that you use the information it's providing you if you can :)
The information it's providing is that "given a use, the use may/must alias with it's def. Anything in between the use and it's current def must be noalias".




IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171206/072382c6/attachment.html>