[PATCH] D53349: [VPlan] Changes to implement VPlan based predication for VPlan-native path.

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 17 14:31:22 PST 2018


fhahn added inline comments.


================
Comment at: lib/Transforms/Vectorize/VPlanPredicator.cpp:46
+  VPValue *IntermediateVal = nullptr;
+  if (ET == EdgeType::TRUE_EDGE)
+    // CurrBB is the true successor of PredBB - nothing to do here.
----------------
sguggill wrote:
> fhahn wrote:
> > If we EdgeType only has TRUE_EDGE and FALSE_EDGE and we turn this into a switch, we will get a nice compiler warning about missing cases if we add more elements to EdgeType.
> I have modified EdgeType to only contain TRUE_EDGE and FALSE_EDGE. However, in the switch I went with a default case where we call llvm_unreachable to get a compiler runtime error if we see an edgetype that is not handled.
IMO it is better to give the compiler the chance to warn/error on this, rather than a runtime check. With the default case, the compiler cannot warn any more, because everything is handled. (There are public bots with warnings as error, which should catch it).


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

https://reviews.llvm.org/D53349





More information about the llvm-commits mailing list