Cleanup of Complex Deinterleaving pass (NFCI)

[NickGuy]

Summary

General cleanup of ComplexDeinterleavingPass; adding more correct checks to make the pass more tolerant, and updating comments to be more accurate.

[dmgreen]

llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp

655–656

I think I find the existing code easier to follow and read. And it is more explicit about matching adds. Neither are super pretty, but I think I would recommend keeping what was there before.

857

This could actually be part of the other patch, if it the first part to introduce single element nodes.

[dmgreen]

This LGTM but please make sure the correct parts are in the correct patches. I don't think this will actually build on its own.

llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp

818

I think this is in the wrong patch now?

[dmgreen]

If you can move the identifySymmetricOperation call to the other patch then what remains LGTM. Thanks.

llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp

for (const auto &Node : CompositeNodes)
       Node->dump(OS);
   }
 
   /// Returns false if the deinterleaving operation should be cancelled for the
   /// current graph.
   bool identifyNodes(Instruction *RootI);
 
   /// Perform the actual replacement of the underlying instruction graph.
-  /// Returns false if the deinterleaving operation should be cancelled for the
-  /// current graph.
   void replaceNodes();
 };
 
 class ComplexDeinterleaving {
 public:
   ComplexDeinterleaving(const TargetLowering *tl, const TargetLibraryInfo *tli)
       : TL(tl), TLI(tli) {}
   bool runOnFunction(Function &F);
std::pair<Instruction *, Instruction *> PartialMatch(
       (Rotation == ComplexDeinterleavingRotation::Rotation_0 ||
        Rotation == ComplexDeinterleavingRotation::Rotation_180)
           ? CommonOperand
           : nullptr,
       (Rotation == ComplexDeinterleavingRotation::Rotation_90 ||
        Rotation == ComplexDeinterleavingRotation::Rotation_270)
           ? CommonOperand
           : nullptr);
-  NodePtr CNode = identifyNodeWithImplicitAdd(
-      cast<Instruction>(CR), cast<Instruction>(CI), PartialMatch);
+
+  auto *CRInst = dyn_cast<Instruction>(CR);
+  auto *CIInst = dyn_cast<Instruction>(CI);
+
+  if (!CRInst || !CIInst) {
+    LLVM_DEBUG(dbgs() << "  - Common operands are not instructions.\n");
+    return nullptr;
+  }
+
+  NodePtr CNode = identifyNodeWithImplicitAdd(CRInst, CIInst, PartialMatch);
   if (!CNode) {
     LLVM_DEBUG(dbgs() << "  - No cnode identified\n");
     return nullptr;
   }
 
   NodePtr UncommonRes = identifyNode(UncommonRealOp, UncommonImagOp);
   if (!UncommonRes) {
     LLVM_DEBUG(dbgs() << "  - No UncommonRes identified\n");
if ((Real->getOpcode() == Instruction::FSub &&
        Imag->getOpcode() == Instruction::Add))
     Rotation = ComplexDeinterleavingRotation::Rotation_90;
   else if ((Real->getOpcode() == Instruction::FAdd &&
             Imag->getOpcode() == Instruction::FSub) ||
            (Real->getOpcode() == Instruction::Add &&
             Imag->getOpcode() == Instruction::Sub))
     Rotation = ComplexDeinterleavingRotation::Rotation_270;
   else {
     LLVM_DEBUG(dbgs() << " - Unhandled case, rotation is not assigned.\n");
     return nullptr;
   }
 
   auto *AR = dyn_cast<Instruction>(Real->getOperand(0));
   auto *BI = dyn_cast<Instruction>(Real->getOperand(1));
   auto *AI = dyn_cast<Instruction>(Imag->getOperand(0));
   auto *BR = dyn_cast<Instruction>(Imag->getOperand(1));
 
   if (!AR || !AI || !BR || !BI) {
if (RealShuffle && ImagShuffle) {
     }
 
     NodePtr PlaceholderNode =
         prepareCompositeNode(llvm::ComplexDeinterleavingOperation::Shuffle,
                              RealShuffle, ImagShuffle);
     PlaceholderNode->ReplacementNode = RealShuffle->getOperand(0);
     return submitCompositeNode(PlaceholderNode);
   }
-  if (RealShuffle || ImagShuffle)
+  if (RealShuffle || ImagShuffle) {
+    LLVM_DEBUG(dbgs() << " - There's a shuffle where there shouldn't be.\n");
     return nullptr;
+  }
 
   auto *VTy = cast<FixedVectorType>(Real->getType());
   auto *NewVTy =
       FixedVectorType::get(VTy->getScalarType(), VTy->getNumElements() * 2);
 
   if (TL->isComplexDeinterleavingOperationSupported(
           ComplexDeinterleavingOperation::CMulPartial, NewVTy) &&
       isInstructionPairMul(Real, Imag)) {
     return identifyPartialMul(Real, Imag);
   }
 
   if (TL->isComplexDeinterleavingOperationSupported(
           ComplexDeinterleavingOperation::CAdd, NewVTy) &&
       isInstructionPairAdd(Real, Imag)) {
     return identifyAdd(Real, Imag);
   }
 
   return nullptr;
 }
 
 bool ComplexDeinterleavingGraph::identifyNodes(Instruction *RootI) {
   Instruction *Real;
   Instruction *Imag;
   if (!match(RootI, m_Shuffle(m_Instruction(Real), m_Instruction(Imag))))
     return false;
 

 
 Value *ComplexDeinterleavingGraph::replaceNode(
     ComplexDeinterleavingGraph::RawNodePtr Node) {
   if (Node->ReplacementNode)
     return Node->ReplacementNode;
 
   Value *Input0 = replaceNode(Node->Operands[0]);
   Value *Input1 = replaceNode(Node->Operands[1]);
   Value *Accumulator =
       Node->Operands.size() > 2 ? replaceNode(Node->Operands[2]) : nullptr;
 
   assert(Input0->getType() == Input1->getType() &&
          "Node inputs need to be of the same type");
 
   Node->ReplacementNode = TL->createComplexDeinterleavingIR(
       Node->Real, Node->Operation, Node->Rotation, Input0, Input1, Accumulator);