[PATCH] D42622: [AggressiveInstCombine] fixed PR36121

[Craig Topper via Phabricator via llvm-commits]

craig.topper accepted this revision.
craig.topper added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: lib/Transforms/AggressiveInstCombine/TruncInstCombine.cpp:335
+      if (Entry != Worklist.end()) {
+        if (auto *NewCI = dyn_cast<TruncInst>(Res))
           *Entry = NewCI;
----------------
aaboud wrote:
> craig.topper wrote:
> > Could these just be isa instead of dyn_cast? Doesn't look like you need a TruncInst to do any of the worklist changes.
> ```
> /// List of all TruncInst instructions to be processed.
>   SmallVector<TruncInst *, 4> Worklist;
> ```
> Unless you are suggesting to change the definition of Worklist to be a list of Instructions, I am not sure I can get rid of the dyn_cast! (I will get a compilation error).
> 
> Saying that, I a prefer to keep Worklist as list of TruncInst, this way I can assume I always have a truncate instruction in this list without need to assert/check that explicitly!
Sorry I searched too quickly and saw the definition of one of the other worklists in this file that uses Value*.


https://reviews.llvm.org/D42622