[PATCH] D49385: [IPSCCP] Run Solve each time we resolved an undef in a function.

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 16 14:58:02 PDT 2018


fhahn added a comment.

In https://reviews.llvm.org/D49385#1164243, @efriedma wrote:

> > Computationally the amount of work we are doing stays the same
>
> I don't think this is right; each call to Solve() has a cost proportional to the size of the module, if I'm not mistaken.


IIUC Solve() only processes instructions in OverdefinedInstWorkList, InstWorkList and BBWorkList. Before calling ResolvedUndefsIn, those should be empty and ResolvedUndefsIn(F) should only add the instruction we resolved a undef for or mark the false successor of a conditional branch on undef executable.

So calling Solve() after resolving undefs should process roughly the same instructions as adding the resolved undef for each function to the worklists. If a discovered return value helps us to get rid of an undef in a later function, we would add a different undef to  the worklist, leading to a slightly different set of instructions visited.

>> Maybe it would make sense to resolve undefs depending on the call graph, e.g. leaf functions first
> 
> We have scc_iterator, but I'm not sure resolving leaf functions first is actually more effective in general.

Yeah I do not think that will be optimal in all cases, I'll give it a try though. Another strategy that might make sense would be resolving the functions with no unknown incoming values/function calls, but I guess in general it's quite tricky.



================
Comment at: test/Transforms/SCCP/ipsccp-basic.ll:253
   %call2 = call i64 @llvm.ctpop.i64(i64 %call1)
   ret void
 ; CHECK-LABEL: define void @test11b
----------------
efriedma wrote:
> What is this supposed to be testing?  ctpop is readnone.
I am not entirely sure. I think what happen before was that we marked `call i64 @test11a()` as overdefined because test11a was unknown, and now we discover the return value of test11a first and can fold llvm.ctpop.i64 based on the known argument.


https://reviews.llvm.org/D49385





More information about the llvm-commits mailing list