[PATCH] D48828: [InstSimplify] fold extracting from std::pair (1/2)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: lib/Analysis/InstructionSimplify.cpp:1289-1291
+  const APInt *ShAmt;
+  if (match(Op1, m_APInt(ShAmt)) &&
+      match(Op0, m_c_Or(m_NUWShl(m_Value(X), m_Specific(Op1)), m_Value(Y)))) {
----------------
inouehrs wrote:
> lebedev.ri wrote:
> > I'm not sure this is better, or the full fix (tests needed.)
> > I would think you'd need
> > ```
> >   const APInt *ShAmt0, *ShAmt1;
> >   if (match(Op1, m_APInt(ShAmt2)) &&
> >       match(Op0, m_c_Or(m_NUWShl(m_Value(X), m_APInt(ShAmt1)), m_Value(Y))) &&
> >       *ShAmt0 == *ShAmt1) {
> >     const APInt *ShAmt = ShAmt1;
> > ```
> Sorry but I cannot catch why you use `m_APInt` in the matcher and then compare the values instead of using `m_Specific`. What kind of code sequences you want to cover with this?
I'm not sure it matters right now.
`m_APInt()` could potentially (not right now) match splat constant with undef's - `<i32 42, i32 undef, i32 42>`
But `m_Specific()` compares the pointers, not the underlying data. So if `ShAmt0` and `ShAmt1` are both splat,
but have different `undef`s (e.g. only one of them has `undef` elements), they would not have the same constant.

Theoretically, that code in my comment would still match this case.
But this does not matter right now since `m_APInt()` does not accept constants with undef elements.


https://reviews.llvm.org/D48828