[PATCH] D146988: [CodeGen] Enable processing of interconnected complex number operations

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Hello. Nick pointed me at the patch but I haven't looked at the details a huge amount. checkNodes feels quite complex, but handling multiple users seems like a nice addition to the pass. The pass should be able to handle multiple basic blocks just fine, but limiting multiple uses to all start in the same block sounds like a good compromise between complexity and functionality.



================
Comment at: llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp:892
+  SmallPtrSet<Instruction *, 16> AllInstructions;
+  SmallVector<Instruction *, 8> ToDo;
+  for (auto *I : OrderedRoots)
----------------
Worklist is a commonly used name in LLVM


================
Comment at: llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp:921
+    for (User *U : I->users()) {
+      if (auto *OpI = dyn_cast<Instruction>(U)) {
+        if (AllInstructions.count(OpI))
----------------
A user of an instruction will always be an instruction.


================
Comment at: llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp:926
+        // Found an instruction that is not used by XCMLA/XCADD chain
+        OuterInstructions.emplace_back(OpI);
+      }
----------------
This seems like it adds the user `OpI` to `OuterInstructions`, just so that later is can look at the operands and add `I` to `ToDo`. Can it just add `I` directly?


================
Comment at: llvm/test/CodeGen/AArch64/complex-deinterleaving-multiuses.ll:93
+;   *p2 = (a * b) * c;
+;   return d * c
+define <4 x float> @monster(<4 x float> %a, <4 x float> %b, <4 x float> %c, <4 x float> %d, ptr %p1, ptr %p2) {
----------------
It is not obvious to me why d * c should prevent the rest of the tree transforming. Can you precommit the tests to show the differences in the review?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146988