[PATCH] D35474: SSAUpdater: Add mode when Phi creation is not allowed

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added a comment.

Hi Daniel, I plan to upload the patch for PR26233 this week. I'd like to think a bit more about relation of recursion and the patch I have - whether it works correctly and whether SSAUpdater work correctly with this pattern.

For now, shortly, the idea of future patch is as follows: in CodeGenPrepare::OptimizeMemoryInst I'd like to collect all cases where address computation is identical but base might be different. So to sink address computation to memory instruction I need to find a common Phi node for all bases which can be used in BB where memory instruction is located. For that purpose I'd like to use SSAUpdater. However it is possible that Phi node for bases may not exists (for example if there is no further uses of base it can be removed).

It is not clear that creation of new Phi node is profitable with respect to folding of address computation to memory instruction.
I'd like to add an option which allows/disallows creation of Phi nodes. For that I need this patch.

SSAUpdater is able to correctly answer to the question: if I know the different values in different basic blocks is there any merge of these values in BB I'm interested in. In general it is not so easy and straightforward in complex CFG.

I also can just use SSAUpdater as is and after that remove the created Phi nodes if needed but this approach seems better to me.

I hope it answers why I'd like to use this utility in the mode when I need only to check whether there is a Phi node merging all my values.



================
Comment at: include/llvm/Transforms/Utils/SSAUpdaterImpl.h:97
+    if (!FindAvailableVals(&BlockList))
+      return Traits::GetUndefVal(BB, Updater);
 
----------------
efriedma wrote:
> This is bad: the caller has no way to tell what happened if you return undef (is the value actually undef at the given point, or did PHI construction fail?).
I see. Is nullptr better?


https://reviews.llvm.org/D35474