[PATCH] D33247: [InstCombine] add isCanonicalPredicate() helper function and use it; NFCI

[Davide Italiano via Phabricator via llvm-commits]

davide added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineInternal.h:95-96
+    return true;
+  }
+}
+
----------------
spatel wrote:
> davide wrote:
> > I think GCC might complain about this so you might consider adding an `llvm_unreachable` at the end of this.
> Do you know of a case where that happens? I couldn't find one:
> https://godbolt.org/g/l0XnXd
> ...so I'd rather not ugly up the code for something if it doesn't really happen. We could 'default: break' and return true outside of the switch if you think that reads better.
I saw it emitting warning in similar cases, but not this one (just checked).
If neither compiler complains, I think you can leave as is (worst case we'll need to fix a builbot failure, not a big deal).


https://reviews.llvm.org/D33247