[CodeGen] Refactor IR generation functions to use IRBuilder in ComplexDeinterleaving pass

[igor.kirillov]

Summary

This patch updates several functions in LLVM's IR generation code to accept
an IRBuilder object as an argument, rather than an Instruction that indicates
the insertion point for new instructions.
This change is necessary to handle sophisticated -Ofast optimization cases
from D148558 where it's unclear which instructions should be used as the
insertion point for new operations.

Depends on D148550

[igor.kirillov]

Rebase

[igor.kirillov]

ping

[NickGuy]

LGTM

llvm/test/CodeGen/AArch64/complex-deinterleaving-mixed-cases.ll

222–228

Any idea what caused these changes? Is it solely down to using a single IRBuilder instance? Or is there some element of non-determinism with regards to the ordering at this point?

[igor.kirillov]

llvm/test/CodeGen/AArch64/complex-deinterleaving-mixed-cases.ll

222–228

While building a new tree consisting of fcmla/fcadd/symmetric from predecessors to successors, it does it without paying attention where the original instructions were. So it caused some irrelevant changes in the order.
It should be deterministic - everything we iterate there uses deterministic structures and maps/sets are only for checking

[igor.kirillov]

Rebase

llvm/include/llvm/CodeGen/TargetLowering.h

virtual bool isComplexDeinterleavingOperationSupported(
       ComplexDeinterleavingOperation Operation, Type *Ty) const {
     return false;
   }
 
   /// Create the IR node for the given complex deinterleaving operation.
   /// If one cannot be created using all the given inputs, nullptr should be
   /// returned.
   virtual Value *createComplexDeinterleavingIR(
-      Instruction *I, ComplexDeinterleavingOperation OperationType,
+      IRBuilderBase &B, ComplexDeinterleavingOperation OperationType,
       ComplexDeinterleavingRotation Rotation, Value *InputA, Value *InputB,
       Value *Accumulator = nullptr) const {
     return nullptr;
   }
 
   //===--------------------------------------------------------------------===//
   // Runtime Library hooks
   //

llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp

private:
   /// complex numbers. There are two ways to represent the Deinterleave
   /// operation:
   /// * Using two shufflevectors with even indices for /pReal instruction and
   /// odd indices for /pImag instructions (only for fixed-width vectors)
   /// * Using two extractvalue instructions applied to `vector.deinterleave2`
   /// intrinsic (for both fixed and scalable vectors)
   NodePtr identifyDeinterleave(Instruction *Real, Instruction *Imag);
 
-  Value *replaceNode(RawNodePtr Node);
+  Value *replaceNode(IRBuilderBase &Builder, RawNodePtr Node);
 
 public:
   void dump() { dump(dbgs()); }
   void dump(raw_ostream &OS) {
     for (const auto &Node : CompositeNodes)
       Node->dump(OS);
   }
 
NodePtr PlaceholderNode =
       prepareCompositeNode(llvm::ComplexDeinterleavingOperation::Deinterleave,
                            RealShuffle, ImagShuffle);
   PlaceholderNode->ReplacementNode = RealShuffle->getOperand(0);
   FinalInstructions.insert(RealShuffle);
   FinalInstructions.insert(ImagShuffle);
   return submitCompositeNode(PlaceholderNode);
 }
 
-static Value *replaceSymmetricNode(ComplexDeinterleavingGraph::RawNodePtr Node,
+static Value *replaceSymmetricNode(IRBuilderBase &B,
+                                   ComplexDeinterleavingGraph::RawNodePtr Node,
                                    Value *InputA, Value *InputB) {
   Instruction *I = Node->Real;
   if (I->isUnaryOp())
     assert(!InputB &&
            "Unary symmetric operations need one input, but two were provided.");
   else if (I->isBinaryOp())
     assert(InputB && "Binary symmetric operations need two inputs, only one "
                      "was provided.");
 
-  IRBuilder<> B(I);
-
   switch (I->getOpcode()) {
   case Instruction::FNeg:
     return B.CreateFNegFMF(InputA, I);
   case Instruction::FAdd:
     return B.CreateFAddFMF(InputA, InputB, I);
   case Instruction::FSub:
     return B.CreateFSubFMF(InputA, InputB, I);
   case Instruction::FMul:
     return B.CreateFMulFMF(InputA, InputB, I);
   }
 
   return nullptr;
 }
 
-Value *ComplexDeinterleavingGraph::replaceNode(
-    ComplexDeinterleavingGraph::RawNodePtr Node) {
+Value *ComplexDeinterleavingGraph::replaceNode(IRBuilderBase &Builder,
+                                               RawNodePtr Node) {
   if (Node->ReplacementNode)
     return Node->ReplacementNode;
 
-  Value *Input0 = replaceNode(Node->Operands[0]);
-  Value *Input1 =
-      Node->Operands.size() > 1 ? replaceNode(Node->Operands[1]) : nullptr;
-  Value *Accumulator =
-      Node->Operands.size() > 2 ? replaceNode(Node->Operands[2]) : nullptr;
+  Value *Input0 = replaceNode(Builder, Node->Operands[0]);
+  Value *Input1 = Node->Operands.size() > 1
+                      ? replaceNode(Builder, Node->Operands[1])
+                      : nullptr;
+  Value *Accumulator = Node->Operands.size() > 2
+                           ? replaceNode(Builder, Node->Operands[2])
+                           : nullptr;
 
   if (Input1)
     assert(Input0->getType() == Input1->getType() &&
            "Node inputs need to be of the same type");
 
   if (Node->Operation == ComplexDeinterleavingOperation::Symmetric)
-    Node->ReplacementNode = replaceSymmetricNode(Node, Input0, Input1);
+    Node->ReplacementNode = replaceSymmetricNode(Builder, Node, Input0, Input1);
   else
     Node->ReplacementNode = TL->createComplexDeinterleavingIR(
-        Node->Real, Node->Operation, Node->Rotation, Input0, Input1,
-        Accumulator);
+        Builder, Node->Operation, Node->Rotation, Input0, Input1, Accumulator);
 
   assert(Node->ReplacementNode && "Target failed to create Intrinsic call.");
   NumComplexTransformations += 1;
   return Node->ReplacementNode;
 }
 
 void ComplexDeinterleavingGraph::replaceNodes() {
   SmallVector<Instruction *, 16> DeadInstrRoots;
   for (auto *RootInstruction : OrderedRoots) {
     // Check if this potential root went through check process and we can
     // deinterleave it
     if (!RootToNode.count(RootInstruction))
       continue;
 
     IRBuilder<> Builder(RootInstruction);
     auto RootNode = RootToNode[RootInstruction];
-    Value *R = replaceNode(RootNode.get());
+    Value *R = replaceNode(Builder, RootNode.get());
     assert(R && "Unable to find replacement for RootInstruction");
     DeadInstrRoots.push_back(RootInstruction);
     RootInstruction->replaceAllUsesWith(R);
   }
 
   for (auto *I : DeadInstrRoots)
     RecursivelyDeleteTriviallyDeadInstructions(I, TLI);
 }

llvm/lib/Target/AArch64/AArch64ISelLowering.h

public:
 
   bool shouldConvertFpToSat(unsigned Op, EVT FPVT, EVT VT) const override;
 
   bool isComplexDeinterleavingSupported() const override;
   bool isComplexDeinterleavingOperationSupported(
       ComplexDeinterleavingOperation Operation, Type *Ty) const override;
 
   Value *createComplexDeinterleavingIR(
-      Instruction *I, ComplexDeinterleavingOperation OperationType,
+      IRBuilderBase &B, ComplexDeinterleavingOperation OperationType,
       ComplexDeinterleavingRotation Rotation, Value *InputA, Value *InputB,
       Value *Accumulator = nullptr) const override;
 
   bool supportSplitCSR(MachineFunction *MF) const override {
     return MF->getFunction().getCallingConv() == CallingConv::CXX_FAST_TLS &&
            MF->getFunction().hasFnAttribute(Attribute::NoUnwind);
   }
   void initializeSplitCSR(MachineBasicBlock *Entry) const override;

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

if ((VTyWidth < 128 && (VTy->isScalableTy() || VTyWidth != 64)) ||
       !llvm::isPowerOf2_32(VTyWidth))
     return false;
 
   return (ScalarTy->isHalfTy() && Subtarget->hasFullFP16()) ||
          ScalarTy->isFloatTy() || ScalarTy->isDoubleTy();
 }
 
 Value *AArch64TargetLowering::createComplexDeinterleavingIR(
-    Instruction *I, ComplexDeinterleavingOperation OperationType,
+    IRBuilderBase &B, ComplexDeinterleavingOperation OperationType,
     ComplexDeinterleavingRotation Rotation, Value *InputA, Value *InputB,
     Value *Accumulator) const {
   VectorType *Ty = cast<VectorType>(InputA->getType());
   bool IsScalable = Ty->isScalableTy();
 
-  IRBuilder<> B(I);
-
   unsigned TyWidth =
       Ty->getScalarSizeInBits() * Ty->getElementCount().getKnownMinValue();
 
   assert(((TyWidth >= 128 && llvm::isPowerOf2_32(TyWidth)) || TyWidth == 64) &&
          "Vector type must be either 64 or a power of 2 that is at least 128");
 
   if (TyWidth > 128) {
     int Stride = Ty->getElementCount().getKnownMinValue() / 2;
     auto *HalfTy = VectorType::getHalfElementsVectorType(Ty);
     auto *LowerSplitA = B.CreateExtractVector(HalfTy, InputA, B.getInt64(0));
     auto *LowerSplitB = B.CreateExtractVector(HalfTy, InputB, B.getInt64(0));
     auto *UpperSplitA =
         B.CreateExtractVector(HalfTy, InputA, B.getInt64(Stride));
     auto *UpperSplitB =
         B.CreateExtractVector(HalfTy, InputB, B.getInt64(Stride));
     Value *LowerSplitAcc = nullptr;
     Value *UpperSplitAcc = nullptr;
     if (Accumulator) {
       LowerSplitAcc = B.CreateExtractVector(HalfTy, Accumulator, B.getInt64(0));
       UpperSplitAcc =
           B.CreateExtractVector(HalfTy, Accumulator, B.getInt64(Stride));
     }
     auto *LowerSplitInt = createComplexDeinterleavingIR(
-        I, OperationType, Rotation, LowerSplitA, LowerSplitB, LowerSplitAcc);
+        B, OperationType, Rotation, LowerSplitA, LowerSplitB, LowerSplitAcc);
     auto *UpperSplitInt = createComplexDeinterleavingIR(
-        I, OperationType, Rotation, UpperSplitA, UpperSplitB, UpperSplitAcc);
+        B, OperationType, Rotation, UpperSplitA, UpperSplitB, UpperSplitAcc);
 
     auto *Result = B.CreateInsertVector(Ty, PoisonValue::get(Ty), LowerSplitInt,
                                         B.getInt64(0));
     return B.CreateInsertVector(Ty, Result, UpperSplitInt, B.getInt64(Stride));
   }
 
   if (OperationType == ComplexDeinterleavingOperation::CMulPartial) {
     if (Accumulator == nullptr)

llvm/lib/Target/ARM/ARMISelLowering.h

public:
 
     bool shouldConvertFpToSat(unsigned Op, EVT FPVT, EVT VT) const override;
 
     bool isComplexDeinterleavingSupported() const override;
     bool isComplexDeinterleavingOperationSupported(
         ComplexDeinterleavingOperation Operation, Type *Ty) const override;
 
     Value *createComplexDeinterleavingIR(
-        Instruction *I, ComplexDeinterleavingOperation OperationType,
+        IRBuilderBase &B, ComplexDeinterleavingOperation OperationType,
         ComplexDeinterleavingRotation Rotation, Value *InputA, Value *InputB,
         Value *Accumulator = nullptr) const override;
 
   protected:
     std::pair<const TargetRegisterClass *, uint8_t>
     findRepresentativeClass(const TargetRegisterInfo *TRI,
                             MVT VT) const override;
 

llvm/lib/Target/ARM/ARMISelLowering.cpp

if (Operation != ComplexDeinterleavingOperation::CAdd)
     return false;
 
   return Subtarget->hasMVEIntegerOps() &&
          (ScalarTy->isIntegerTy(8) || ScalarTy->isIntegerTy(16) ||
           ScalarTy->isIntegerTy(32));
 }
 
 Value *ARMTargetLowering::createComplexDeinterleavingIR(
-    Instruction *I, ComplexDeinterleavingOperation OperationType,
+    IRBuilderBase &B, ComplexDeinterleavingOperation OperationType,
     ComplexDeinterleavingRotation Rotation, Value *InputA, Value *InputB,
     Value *Accumulator) const {
 
   FixedVectorType *Ty = cast<FixedVectorType>(InputA->getType());
 
-  IRBuilder<> B(I);
-
   unsigned TyWidth = Ty->getScalarSizeInBits() * Ty->getNumElements();
 
   assert(TyWidth >= 128 && "Width of vector type must be at least 128 bits");
 
   if (TyWidth > 128) {
     int Stride = Ty->getNumElements() / 2;
     auto SplitSeq = llvm::seq<int>(0, Ty->getNumElements());
     auto SplitSeqVec = llvm::to_vector(SplitSeq);
     ArrayRef<int> LowerSplitMask(&SplitSeqVec[0], Stride);
     ArrayRef<int> UpperSplitMask(&SplitSeqVec[Stride], Stride);
 
     auto *LowerSplitA = B.CreateShuffleVector(InputA, LowerSplitMask);
     auto *LowerSplitB = B.CreateShuffleVector(InputB, LowerSplitMask);
     auto *UpperSplitA = B.CreateShuffleVector(InputA, UpperSplitMask);
     auto *UpperSplitB = B.CreateShuffleVector(InputB, UpperSplitMask);
     Value *LowerSplitAcc = nullptr;
     Value *UpperSplitAcc = nullptr;
 
     if (Accumulator) {
       LowerSplitAcc = B.CreateShuffleVector(Accumulator, LowerSplitMask);
       UpperSplitAcc = B.CreateShuffleVector(Accumulator, UpperSplitMask);
     }
 
     auto *LowerSplitInt = createComplexDeinterleavingIR(
-        I, OperationType, Rotation, LowerSplitA, LowerSplitB, LowerSplitAcc);
+        B, OperationType, Rotation, LowerSplitA, LowerSplitB, LowerSplitAcc);
     auto *UpperSplitInt = createComplexDeinterleavingIR(
-        I, OperationType, Rotation, UpperSplitA, UpperSplitB, UpperSplitAcc);
+        B, OperationType, Rotation, UpperSplitA, UpperSplitB, UpperSplitAcc);
 
     ArrayRef<int> JoinMask(&SplitSeqVec[0], Ty->getNumElements());
     return B.CreateShuffleVector(LowerSplitInt, UpperSplitInt, JoinMask);
   }
 
   auto *IntTy = Type::getInt32Ty(B.getContext());
 
   ConstantInt *ConstRotation = nullptr;

llvm/test/CodeGen/AArch64/complex-deinterleaving-mixed-cases.ll

entry:
   ret <4 x float> %interleaved.vec
 }
 
 ; Expected to transform
 define <4 x float> @mul_add90_mul(<4 x float> %a, <4 x float> %b, <4 x float> %c) {
 ; CHECK-LABEL: mul_add90_mul:
 ; CHECK:       // %bb.0: // %entry
 ; CHECK-NEXT:    movi v3.2d, #0000000000000000
 ; CHECK-NEXT:    movi v4.2d, #0000000000000000
-; CHECK-NEXT:    fcmla v3.4s, v1.4s, v0.4s, #0
-; CHECK-NEXT:    fcmla v4.4s, v2.4s, v0.4s, #0
-; CHECK-NEXT:    fcmla v3.4s, v1.4s, v0.4s, #90
-; CHECK-NEXT:    fcmla v4.4s, v2.4s, v0.4s, #90
-; CHECK-NEXT:    fcadd v0.4s, v4.4s, v3.4s, #90
+; CHECK-NEXT:    fcmla v3.4s, v2.4s, v0.4s, #0
+; CHECK-NEXT:    fcmla v4.4s, v1.4s, v0.4s, #0
+; CHECK-NEXT:    fcmla v3.4s, v2.4s, v0.4s, #90
+; CHECK-NEXT:    fcmla v4.4s, v1.4s, v0.4s, #90
+; CHECK-NEXT:    fcadd v0.4s, v3.4s, v4.4s, #90
 ; CHECK-NEXT:    ret
 entry:
   %ar = shufflevector <4 x float> %a, <4 x float> poison, <2 x i32> <i32 0, i32 2>
   %ai = shufflevector <4 x float> %a, <4 x float> poison, <2 x i32> <i32 1, i32 3>
   %br = shufflevector <4 x float> %b, <4 x float> poison, <2 x i32> <i32 0, i32 2>
   %bi = shufflevector <4 x float> %b, <4 x float> poison, <2 x i32> <i32 1, i32 3>
   %cr = shufflevector <4 x float> %c, <4 x float> poison, <2 x i32> <i32 0, i32 2>
   %ci = shufflevector <4 x float> %c, <4 x float> poison, <2 x i32> <i32 1, i32 3>