[PATCH] D84270: [SCCP] Remove dead switch cases based on range information

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 27 03:26:02 PDT 2020


nikic marked an inline comment as done.
nikic added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/SCCP.cpp:658
 
-    if (!CI) {   // Overdefined or unknown condition?
-      // All destinations are executable!
-      if (!SCValue.isUnknownOrUndef())
-        Succs.assign(TI.getNumSuccessors(), true);
+    if (SCValue.isConstantRange()) {
+      const ConstantRange &Range = SCValue.getConstantRange();
----------------
fhahn wrote:
> nikic wrote:
> > fhahn wrote:
> > > Do we have to exclude constant ranges that may be undef here? Should all successors be feasible in that case?
> > Depends on how conservative we want to be :) Per langref, switch on undef is UB. In practical terms, I would expect the situation for switch to be less problematic than for br, because we generally don't create switches "out of thin air", e.g. loop unswitch does not create switch statements.
> I think it would be more consistent to follow to same logic as we do for branches here and flip the switch together. It might not be as big of a problem as for branches and there's no clear line, but it might be surprising for readers of the code. And we should really work towards flipping globally :)
Makes sense. In that case I'll wait on the assume change in D83643 (or I could split that off into a separate revision), to allow easy testing.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84270/new/

https://reviews.llvm.org/D84270





More information about the llvm-commits mailing list