[PATCH] D18710: code hoisting using GVN

[Daniel Berlin via llvm-commits]

dberlin added a subscriber: dberlin.
dberlin added a comment.

1. This needs a ton more correctness tests. It also needs real performance numbers, both on the compile time and execution time side.

2. For loads, i have trouble seeing how it gets the following case right:

    1
   / \
  2   3
  |   |
  4   5

Two loads, one in 4, one in 5.
All pointer operands are defined in block 1 (so you are all good there).
Both 2 and 3 contain calls that kill memory.
It looks like you will try to hoist to 1 anyway, because nowhere do you use memory dependence or memoryssa to figure out *if the memory state* is the same. For scalars, it doesn't matter.

You do stop trying to hoist when you hit modifying expressions in 4 or 5, but that won't help you here.

3. This seems like a really expensive way of doing this :)

This is pretty badly N^2.

Can i suggest a different method, closely based on what we do in GCC for speed reasons (You will discover the vast majority of things have only one expression for each VN, so it's pointless to walk and do lookups on them)

Make a multimap from VN to each expression with that VN (VNTable is not currently this) over the entire program.

  For each VN in the table:
   if (size (expressions with a given VN) > 1):
     One of:
      A. Calculate VBE over the expressions with that VN.
      B. Do something similar to what you do now (now you have N^2 where N is the number of expressions with a given VN, instead of number of blocks)
      C. Something like this should work for completeness and sparseness:
          For each block in domtree in DFS order:
                For each expression, if DFSin/DFSOut(expression->parent) within range of DFSin/DFSOut(domtree), push(possiblehoistlist (a list), expression (current expression), block (insertion point))
                If you have 2 or more things on possiblehoistlist, calculate availability of operands for each expression in block (you can cache the highest point you can move them to for later to checking again and again. Since you are using dominance, you know they can only be hoisted to blocks dominated by the highest point you can hoist to).
                If 2 or more things are still available, hoist

Note you can also likely skip any domtree block that does not have two or more children, i just haven't proven it to myself yet.


Repository:
  rL LLVM

http://reviews.llvm.org/D18710