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

[Hiroshi Inoue via Phabricator via llvm-commits]

inouehrs marked 3 inline comments as done.
inouehrs added inline comments.


================
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
----------------
lebedev.ri wrote:
> 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.
> ```
Is this better? I mention std::pair as an example.


================
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:
> lebedev.ri wrote:
> > Extra unneeded brace `()`
> What about commutativity? This should be `m_c_Or`.
Modified. Thank you for pointing this out. 


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


https://reviews.llvm.org/D48828