[PATCH] D56214: AggressiveInstCombine: Fold full mul i64 x i64 -> i128

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

Haven't taken a deep look yet, but some preliminary thoughts.
Also, i don't think this should be hardcoded to some particular bitwidth.



================
Comment at: lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp:255-265
+template <typename LHS>
+inline BinaryOp_match<LHS, specific_intval, Instruction::And>
+m_LowPart(const LHS &L) {
+  return m_And(L, m_SpecificInt(0xffffffff));
+}
+
+template <typename LHS>
----------------
I don't see why these need to be actual functions, lambdas will do?


================
Comment at: lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp:312-316
+  if (match(&I, m_c_Or(m_LowPart(m_Value(t0)),
+                       m_Shl(m_c_Add(m_LowPart(m_c_Add(m_HighPart(m_Value(t0a)),
+                                                       m_Value(t1))),
+                                     m_Value(t2)),
+                             m_SpecificInt(32))))) {
----------------
```
  if (match(&I, m_c_Or(m_LowPart(m_Value(t0)),
                       m_Shl(m_c_Add(m_LowPart(m_c_Add(m_HighPart(m_Deferred(t0)),
                                                       m_Value(t1))),
                                     m_Value(t2)),
                             m_SpecificInt(32))))) {
```


================
Comment at: lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp:331
+    // 4. For t0 require to have the same arguments as t1.
+    if (t0 == t0a &&
+        match(t1, m_c_Mul(m_HighPart(m_Value(x)), m_LowPart(m_Value(y)))) &&
----------------
and now you only have `t0`, no `t0a`


================
Comment at: test/Transforms/AggressiveInstCombine/mul128.ll:1
+; RUN: opt < %s -aggressive-instcombine -S | FileCheck %s
+
----------------
Please use `llvm/utils/update_test_checks.py`.
And move the initial test case into another review, so this diff shows the *change* in the test output.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56214