[PATCH] D48828: [InstSimplify] fold extracting from std::pair

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a reviewer: lebedev.ri.
lebedev.ri added a comment.

The InstSimplify change needs it's own test set in `test/Transforms/InstSimplify`.



================
Comment at: lib/Analysis/InstructionSimplify.cpp:1274-1278
+  // If the operation is extracting a member from std::pair,
+  // we can bypass make_pair.
+  // def make_pair(X,Y) := (X << ShiftCnt) | Y
+  //     assuming sizeof(X) + ShiftCnt <= OpWidth and sizeof(Y) <= ShiftCnt
+  // make_pair(X,Y).first = ((X << ShiftCnt) | Y) >> ShiftCnt -> X
----------------
I think this shouldn't talk about C++ here. Just talk about IR.
```
// Given  Op0 l>> Op1.
// If Op1 is a constant, and Op0 is  (X nuw<< Op1) | Y
// If Y l>> Op1 == 0. we can extract `X` without extra instructions.
```


================
Comment at: lib/Analysis/InstructionSimplify.cpp:1281
+  if (isa<ConstantInt>(Op1) &&
+      (match(Op0, m_Or(m_NUWShl(m_Value(X), m_Specific(Op1)), m_Value(Y))))) {
+    KnownBits XKnown = computeKnownBits(X, Q.DL, 0, Q.AC, Q.CxtI, Q.DT);
----------------
Extra unneeded brace `()`


================
Comment at: lib/Analysis/InstructionSimplify.cpp:1281
+  if (isa<ConstantInt>(Op1) &&
+      (match(Op0, m_Or(m_NUWShl(m_Value(X), m_Specific(Op1)), m_Value(Y))))) {
+    KnownBits XKnown = computeKnownBits(X, Q.DL, 0, Q.AC, Q.CxtI, Q.DT);
----------------
lebedev.ri wrote:
> Extra unneeded brace `()`
What about commutativity? This should be `m_c_Or`.


================
Comment at: lib/Analysis/InstructionSimplify.cpp:1286
+    unsigned Width = Op0->getType()->getScalarSizeInBits();
+    unsigned WidthX = Width - XKnown.countMinLeadingZeros();
+    unsigned WidthY = Width - YKnown.countMinLeadingZeros();
----------------
Why do we check this? I think `nuw` already implies that.


https://reviews.llvm.org/D48828