[PATCH] D18710: code hoisting using GVN

[Sebastian Pop via llvm-commits]

sebpop added a comment.

Thanks Danny for your feedback.

In http://reviews.llvm.org/D18710#391283, @dberlin wrote:

> 1. This needs a ton more correctness tests.


We will add more testcases.

> It also needs real performance numbers, both on the compile time and execution time side.


We ran the llvm nightly test-suite on x86, and we have not seen any meaningful numbers we could report: it is too noisy because of the short execution times. We will run spec 2k and 2k6 on x86 where things should be more stable.

> 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.


The pattern we are matching does not match this example: please correct me if I'm wrong here.
The only case we are handling is without the intermediate blocks 2 and 3 on your example:

  BasicBlock *BB = Dom->getBlock();
  // Only handle two branches for now: it is possible to extend the hoisting
  // to switch statements.
  BranchInst *BI = dyn_cast<BranchInst>(BB->getTerminator());
  if (!BI || BI->getNumSuccessors() != 2)
    return false;
  
  BasicBlock *BB1 = BI->getSuccessor(0);
  BasicBlock *BB2 = BI->getSuccessor(1);
  assert(BB1 != BB2 && "invalid CFG");
  
  if (!DT->properlyDominates(BB, BB1) ||
      !DT->properlyDominates(BB, BB2) ||
      BB1->isEHPad() || BB1->hasAddressTaken() ||
      BB2->isEHPad() || BB2->hasAddressTaken())
    return false;

BB1 and BB2 are the direct successors of BB: there is no other block in between.
We only hoist expressions from BB1 and BB2 into BB.

> 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


We will implement your suggestion: Aditya has also remarked that we could be more efficient on compile time by using the internal structures of the VN table, though I agree with you that we will need some more changes.


Repository:
  rL LLVM

http://reviews.llvm.org/D18710