[PATCH] D52707: Switch optimization in IR for known maximum switch values

Hans Wennborg via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 23 05:20:14 PDT 2018


hans added inline comments.


================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:4884
+                         DefaultResultsList, DL, TTI))
+      return false;
+
----------------
ayonam wrote:
> hans wrote:
> > ayonam wrote:
> > > hans wrote:
> > > > What's happening here? Why do we need to find a constant value on the phi node?
> > > > 
> > > > We'll need to update any phi node in the default block when adding the new case, but there's no need for it to be constant.
> > > It should just be a Value and not a Constant.  I had a test case that had a constant on the PHI node and hence I had coded thus.
> > > 
> > > I tried changing this to a Value and realized that there isn't a method that gives me all the Values that are created in a given Case block.  There is a method that returns all constants that are created in a Case block.  Would you know of any other method that would return me the Values?  Or else I will have to write one of my own.
> > > 
> > > 
> > What I'm saying is there should be no need to compute the resulting value at phi nodes at all. That stuff is used when replacing switches with a table of constant values, it does not apply here.
> We need to get all the PHI nodes resulting from this switch and add an incoming edge to all those PHI nodes for every case that we add to this switch.  Else the block containing the PHIs will have more predecessors than the number of incoming edges to the PHIs.  I'm trying to do essentially that.  Could you please suggest a better way to achieve this?
Yes, the phi nodes in DefaultBlock need to be updated, but this is not the way to do it. I think something like

```
AddPredecessorToBlock(DefaultBlock, SI->getParent(), SI->getParent())
```

when adding the cases below should work.


https://reviews.llvm.org/D52707





More information about the llvm-commits mailing list