[PATCH] D31583: StackColoring: smarter check for slot overlap

[Than McIntosh via Phabricator via llvm-commits]

thanm added a comment.

In https://reviews.llvm.org/D31583#754945, @arielb1 wrote:

> Ready and just waiting for review.


Thanks for the ping/reminder.

I read through Björn's April 17th comment. The explanation makes sense, but this is something that should be in the code as opposed to only in a Phab review. Please add something along these lines to the "Implementation Notes" command section -- that is where other material exists relating to how the data flow analysis works.

My existing comments regarding naming still stand (overloading of the terms "active" and naming of the BB set "Use").

Ditto on my previous comment regarding testing. I recommend running a clang/llvm bootstrap build to make sure you haven't broken anything. You can find instructions on how to do bootstrap builds at http://llvm.org/docs/AdvancedBuilds.html. This is purely an advisory suggestion, you are free to ignore it (I mention it mainly since when I made changes to stack coloring previously it was helpful in flushing out problems that I hadn't anticipated).


https://reviews.llvm.org/D31583