[PATCH] D37163: [LICM] sink through non-trivially replicable PHI

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added a comment.

Overall, i'm unclear  on why you can't compute which ones will actually be sunk, and then split the preds.
Also, the examples does not even require splitting preds to effect the transform.

These are simply phi of ops form, and there is an equivalent op of phis form you could use directly in the exit block.
For example, you have, in the last test (i converted the checks to the code):

  Out12.split.loop.exit:
  lcssaphi1 = phi i32 [ %N_addr.0.pn, %ContLoop ]
  mulres  = mul i32 %N, %lcssaphi1
  subres1 = sub %mulres, %N2
  br label %Out12
  
  Out12.split.loop.exit1:
  lcssaphi2 = phi i32 [ %N_addr.0.pn, %Loop ]
  mulres2 =  mul i32 %N, %lcssaphi2
  subres2 = sub i32, %mulres2, %N
  br label %Out12
  
  Out12:
  something = phi i32 [%subres, %Out12.split.loop.exit ], [ %subres2, %Out12.split.loop.exit1 ]

This is the same as

  Out12: 
  phires = phi i32 [%N, %Loop], [%N2, %ContLoop]
  mulres = mul i32 %N, %N_addr.0.pn
  ; something here has the same value as the phi named something above
  something = sub i32 %mulres, %phires

It seems to me if you can compute which can be sunk before choosing to split the preds, you can do it without splitting the preds.
What am i missing?



================
Comment at: lib/Transforms/Scalar/LICM.cpp:911
+    UE = I.user_end();
+  }
 
----------------
Is there a reason not to just put all the users in a smallvector of WeakVH first?
While it's true splitPred may invalidate the iterators, none of the answers to what this loop does can change (that i can see) as a result of that (nor will this loop do anything for any new values).



https://reviews.llvm.org/D37163