[PATCH] D69245: [InstCombine] Canonicalize uadd.with.overflow to uadd.sat

[Dave Green via Phabricator via llvm-commits]

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


================
Comment at: llvm/include/llvm/IR/PatternMatch.h:561-563
+/// Match an instruction of type T, capturing it if we match.
+/// For example m_isa<WithOverflowInst>(Y)
+template <typename T> inline bind_ty<T> m_isa(T *&I) { return I; }
----------------
lebedev.ri wrote:
> nikic wrote:
> > lebedev.ri wrote:
> > > I don't like this one.
> > > Can we change it to be a two-step process - just make `m_isa<>()` a predicate,
> > > if `isa<>()` matches, then it should apply inner matcher, if any.
> > > And said inner matcher can be `m_Value()`.
> > I'm not sure I understand how that would be used. If you use something like `m_isa<WithOverflowInst>(m_Value(V))`, wouldn't `V` have to be `Value *` rather than `WithOverflowInst *`? That seems less useful to me. It's also not clear what things apart from `m_Value()` you would use as the inner matcher.
> Good point. I'm mainly unhappy about the name.
> `m_isa<>()` really looks similar to normal `isa<>()`, from looking at it i would not
> have guessed it takes reference to the pointer of that type and binds to it if matched.
Right. I did think of that but is seemed OK as it was in the middle of a match statement.

Got a better suggestion, or a preference out of these?
```
m_a<WithOverflowInst>(II)
m_InstanceOf<WithOverflowInst>(II)
m_OfType<WithOverflowInst>(II)
m_OneOfTheseThings<WithOverflowInst>(II)
```


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

https://reviews.llvm.org/D69245