[Review]: Sink multiple instructions per iteration in InstCombine

Aditya Nandakumar aditya_nandakumar at apple.com
Fri Jun 27 13:17:38 PDT 2014


Thanks Chandler for your review.
The problem I saw in a few test cases was that instcombine had finished visiting all instructions (Iteration#0) and the only possible changes were sinking of instructions. This sinking of one instruction opened up sinking opportunities for other instructions which were only being used by the current(sunk) instruction. Since we are visiting top-down, there is only one instruction being sunk per iteration.
So in some my test cases, instcombine ran for 8 iterations where in iterations 1-8, it sank one instruction per iteration. The test cases are about 150-300 lines long and we are visiting all those instructions every iteration even though the only change possible is the sinking.

Consider the following example. 
bb1: %d0 = ...
	%d1 = .. %d0..
	%d2 = .. %d1..
	%d3 = op %d2 ..
   	...
bb2:
      .. =  op %d3

Currently instcombine would sink d3 in Iteration#0, d2 in Iteration#1, d1 in Iteration#2 and d0 in Iteration#3 - but it only requires one iteration in all.

Updated patch - removed C style comments and fixed typo(I think).

> On Jun 26, 2014, at 5:54 PM, Chandler Carruth <chandlerc at google.com> wrote:
> 
> I don't immediately understand why this is such a performance improvement here (and not elsewhere in instcombine)... Do you have any insight into that? Is it a locality problem that we don't re-visit the other sinkable instructions in the right order? It feels pretty weird to have a worklist-inside-a-worklist, so I'm just trynig to understand it better...
> 
> Also, the patch has a bunch of typos in comments and uses C-style comments...
> 
> 
> On Mon, Jun 23, 2014 at 10:57 PM, Aditya Nandakumar <aditya_nandakumar at apple.com> wrote:
> Hi All
> 
> I want to make a small optimization for instcombine. While sinking instructions (with single use) to the use block, also check if any other instructions which are used in the current instruction (operands) can be sunk.
> 
> This speeds up InstCombine time in several cases as previously only one instruction would be sunk per iteration.
> 
> Please review
> 
> Thanks
> Aditya
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140627/a0353ab8/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OSInstCombineSinkMultipleInsts.patch
Type: application/octet-stream
Size: 4164 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140627/a0353ab8/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140627/a0353ab8/attachment-0001.html>


More information about the llvm-commits mailing list