[PATCH] Use DomTree in MachineSink to sink over diamonds.

[Patrik Hägglund H]

Thanks! Commited as r223350.

/Patrik Hägglund

-----Original Message-----
From: Hal Finkel [mailto:hfinkel at anl.gov] 
Sent: den 4 december 2014 02:16
To: Patrik Hägglund H
Cc: Jonas Paulsson; llvm-commits at cs.uiuc.edu
Subject: Re: [PATCH] Use DomTree in MachineSink to sink over diamonds.

----- Original Message -----
> From: "Patrik Hägglund H" <patrik.h.hagglund at ericsson.com>
> To: llvm-commits at cs.uiuc.edu
> Cc: "Jonas Paulsson" <jonas.paulsson at ericsson.com>
> Sent: Wednesday, December 3, 2014 7:08:27 AM
> Subject: [PATCH] Use DomTree in MachineSink to sink over diamonds.
> 
> This is a patch provided by Jonas Paulsson.
> 

LGTM.

+        if (DTChild->getIDom()->getBlock() == MI->getParent() &&
+            !MBB->isSuccessor(DTChild->getBlock()))

Please add a comment next to the isSuccessor check explaining why it is being done -- to prevent adding MBBs to the Succs array that are already there (if you don't realize that the direct successors have already been added, this check is non-obvious, which is why I'd like the comment).

Thanks again,
Hal

> 
> 
> @ Manman Ren: Can you check the change in test/ CodeGen /X86/
> ragreedy -hoist- spill.ll ?
> 
> 
> 
> 
> 
> According to a previous FIXME comment we now not only look at MBB
> 
> successors, but also handle code sinking past them:
> 
> 
> 
> x = computation
> 
> if () {} else {}
> 
> use x
> 
> 
> 
> The instruction could be sunk over the whole diamond for the
> 
> if/then/else (or loop, etc ), allowing it to be sunk into other
> blocks
> 
> after that.
> 
> 
> 
> Modified test added in r204522, due to one spill less present.
> 
> 
> 
> Minor fixes in comments.
> 
> ---
> 
> lib/ CodeGen /MachineSink.cpp | 34 ++++++++++++++++++--------------
> 
> test/ CodeGen /X86/ ragreedy -hoist- spill.ll | 1 -
> 
> 2 files changed, 19 insertions(+), 16 deletions(-)
> 
> 
> 
> 
> 
> /Patrik Hägglund
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory