[ARM][ParallelDSP] Fix for sext input

[samparker]

Summary

The incoming accumulator value can be discovered through a sext, in which case there will be a mismatch between the input and the result. So sign extend the accumulator input if we're performing a 64-bit mac.

[efriedma]

lib/Target/ARM/ARMParallelDSP.cpp

137

Is this really the right place to do this? At this point, we still aren't sure we're going to transform the code. You could instead just do this after you call getAccumulator() in InsertParallelMACs.

test/CodeGen/ARM/ParallelDSP/blocks.ll

1

Why -dce?

[samparker]

lib/Target/ARM/ARMParallelDSP.cpp

137

good point.

test/CodeGen/ARM/ParallelDSP/blocks.ll

1

Just because it makes it easier for me to write the tests and read the output.

[samparker]

Moved the fix to where all the accumulator handling is performed.

[efriedma]

LGTM

lib/Target/ARM/ARMParallelDSP.cpp

bool InsertAcc(Value *V) {
         return false;
       Acc = V;
       return true;
     }
 
     /// Set two MulCandidates, rooted at muls, that can be executed as a single
     /// parallel operation.
     void AddMulPair(MulCandidate *Mul0, MulCandidate *Mul1,
                     bool Exchange = false) {
       LLVM_DEBUG(dbgs() << "Pairing:\n"
                  << *Mul0->Root << "\n"
                  << *Mul1->Root << "\n");
       Mul0->Paired = true;
       Mul1->Paired = true;
       if (Exchange)
         Mul1->Exchange = true;
       MulPairs.push_back(std::make_pair(Mul0, Mul1));
     }
 
     /// Return true if enough mul operations are found that can be executed in
     /// parallel.
     bool CreateParallelPairs();
 
     /// Return the add instruction which is the root of the reduction.
     Instruction *getRoot() { return Root; }
 
     bool is64Bit() const { return Root->getType()->isIntegerTy(64); }
 
+    Type *getType() const { return Root->getType(); }
+
     /// Return the incoming value to be accumulated. This maybe null.
     Value *getAccumulator() { return Acc; }
 
     /// Return the set of adds that comprise the reduction.
     SetVector<Instruction*> &getAdds() { return Adds; }
 
     /// Return the MulCandidate, rooted at mul instruction, that comprise the
     /// the reduction.
void ARMParallelDSP::InsertParallelMACs(Reduction &R) {
   MulCandList &MulCands = R.getMuls();
   for (auto &MulCand : MulCands) {
     if (MulCand->Paired)
       continue;
 
     Value *Mul = MulCand->Root;
     LLVM_DEBUG(dbgs() << "Accumulating unpaired mul: " << *Mul << "\n");
 
-    if (R.getRoot()->getType() != Mul->getType()) {
+    if (R.getType() != Mul->getType()) {
       assert(R.is64Bit() && "expected 64-bit result");
-      Mul = Builder.CreateSExt(Mul, R.getRoot()->getType());
+      Mul = Builder.CreateSExt(Mul, R.getType());
     }
 
     if (!Acc) {
       Acc = Mul;
       continue;
     }
 
     Acc = Builder.CreateAdd(Mul, Acc);
     InsertAfter = cast<Instruction>(Acc);
   }
 
-  if (!Acc)
+  if (!Acc) {
     Acc = R.is64Bit() ?
       ConstantInt::get(IntegerType::get(M->getContext(), 64), 0) :
       ConstantInt::get(IntegerType::get(M->getContext(), 32), 0);
+  } else if (Acc->getType() != R.getType()) {
+    Builder.SetInsertPoint(R.getRoot());
+    Acc = Builder.CreateSExt(Acc, R.getType());
+  }
 
   IntegerType *Ty = IntegerType::get(M->getContext(), 32);
   for (auto &Pair : R.getMulPairs()) {
     MulCandidate *LHSMul = Pair.first;
     MulCandidate *RHSMul = Pair.second;
     LoadInst *BaseLHS = LHSMul->getBaseLoad();
     LoadInst *BaseRHS = RHSMul->getBaseLoad();
     LoadInst *WideLHS = WideLoads.count(BaseLHS) ?

test/CodeGen/ARM/ParallelDSP/blocks.ll

-; RUN: opt -arm-parallel-dsp -mtriple=armv7-a -S %s -o - | FileCheck %s
+; RUN: opt -arm-parallel-dsp -dce -mtriple=armv7-a -S %s -o - | FileCheck %s
 
 ; CHECK-LABEL: single_block
 ; CHECK: [[CAST_A:%[^ ]+]] = bitcast i16* %a to i32*
 ; CHECK: [[A:%[^ ]+]] = load i32, i32* [[CAST_A]]
 ; CHECK: [[CAST_B:%[^ ]+]] = bitcast i16* %b to i32*
 ; CHECK: [[B:%[^ ]+]] = load i32, i32* [[CAST_B]]
 ; CHECK: call i32 @llvm.arm.smlad(i32 [[A]], i32 [[B]], i32 %acc)
 define i32 @single_block(i16* %a, i16* %b, i32 %acc) {

test/CodeGen/ARM/ParallelDSP/sext-acc.ll

+; RUN: opt -arm-parallel-dsp -dce -mtriple=armv7-a -S %s -o - | FileCheck %s
+
+; CHECK-LABEL: sext_acc_1
+; CHECK: [[CAST_A:%[^ ]+]] = bitcast i16* %a to i32*
+; CHECK: [[A:%[^ ]+]] = load i32, i32* [[CAST_A]]
+; CHECK: [[CAST_B:%[^ ]+]] = bitcast i16* %b to i32*
+; CHECK: [[B:%[^ ]+]] = load i32, i32* [[CAST_B]]
+; CHECK: [[ACC:%[^ ]+]] = sext i32 %acc to i64
+; CHECK: call i64 @llvm.arm.smlald(i32 [[A]], i32 [[B]], i64 [[ACC]])
+define i64 @sext_acc_1(i16* %a, i16* %b, i32 %acc) {
+entry:
+  %ld.a.0 = load i16, i16* %a
+  %sext.a.0 = sext i16 %ld.a.0 to i32
+  %ld.b.0 = load i16, i16* %b
+  %sext.b.0 = sext i16 %ld.b.0 to i32
+  %mul.0 = mul i32 %sext.a.0, %sext.b.0
+  %addr.a.1 = getelementptr i16, i16* %a, i32 1
+  %addr.b.1 = getelementptr i16, i16* %b, i32 1
+  %ld.a.1 = load i16, i16* %addr.a.1
+  %sext.a.1 = sext i16 %ld.a.1 to i32
+  %ld.b.1 = load i16, i16* %addr.b.1
+  %sext.b.1 = sext i16 %ld.b.1 to i32
+  %mul.1 = mul i32 %sext.a.1, %sext.b.1
+  %sext.mul.0 = sext i32 %mul.0 to i64
+  %sext.mul.1 = sext i32 %mul.1 to i64
+  %add = add i64 %sext.mul.0, %sext.mul.1
+  %sext.acc = sext i32 %acc to i64
+  %res = add i64 %add, %sext.acc
+  ret i64 %res
+}
+
+; CHECK-LABEL: sext_acc_2
+; CHECK: [[CAST_A:%[^ ]+]] = bitcast i16* %a to i32*
+; CHECK: [[A:%[^ ]+]] = load i32, i32* [[CAST_A]]
+; CHECK: [[CAST_B:%[^ ]+]] = bitcast i16* %b to i32*
+; CHECK: [[B:%[^ ]+]] = load i32, i32* [[CAST_B]]
+; CHECK: [[CAST_A_2:%[^ ]+]] = bitcast i16* %addr.a.2 to i32*
+; CHECK: [[A_2:%[^ ]+]] = load i32, i32* %4
+; CHECK: [[CAST_B_2:%[^ ]+]] = bitcast i16* %addr.b.2 to i32*
+; CHECK: [[B_2:%[^ ]+]] = load i32, i32* %6
+; CHECK: [[ACC:%[^ ]+]] = sext i32 %acc to i64
+; CHECK: [[SMLALD:%[^ ]+]] = call i64 @llvm.arm.smlald(i32 [[A]], i32 [[B]], i64 [[ACC]])
+; CHECK: call i64 @llvm.arm.smlald(i32 [[A_2]], i32 [[B_2]], i64 [[SMLALD]])
+define i64 @sext_acc_2(i16* %a, i16* %b, i32 %acc) {
+entry:
+  %ld.a.0 = load i16, i16* %a
+  %sext.a.0 = sext i16 %ld.a.0 to i32
+  %ld.b.0 = load i16, i16* %b
+  %sext.b.0 = sext i16 %ld.b.0 to i32
+  %mul.0 = mul i32 %sext.a.0, %sext.b.0
+  %addr.a.1 = getelementptr i16, i16* %a, i32 1
+  %addr.b.1 = getelementptr i16, i16* %b, i32 1
+  %ld.a.1 = load i16, i16* %addr.a.1
+  %sext.a.1 = sext i16 %ld.a.1 to i32
+  %ld.b.1 = load i16, i16* %addr.b.1
+  %sext.b.1 = sext i16 %ld.b.1 to i32
+  %mul.1 = mul i32 %sext.a.1, %sext.b.1
+  %sext.mul.0 = sext i32 %mul.0 to i64
+  %sext.mul.1 = sext i32 %mul.1 to i64
+  %add = add i64 %sext.mul.0, %sext.mul.1
+  %sext.acc = sext i32 %acc to i64
+  %add.1 = add i64 %add, %sext.acc
+  %addr.a.2 = getelementptr i16, i16* %a, i32 2
+  %addr.b.2 = getelementptr i16, i16* %b, i32 2
+  %ld.a.2 = load i16, i16* %addr.a.2
+  %sext.a.2 = sext i16 %ld.a.2 to i32
+  %ld.b.2 = load i16, i16* %addr.b.2
+  %sext.b.2 = sext i16 %ld.b.2 to i32
+  %mul.2 = mul i32 %sext.a.2, %sext.b.2
+  %sext.mul.2 = sext i32 %mul.2 to i64
+  %addr.a.3 = getelementptr i16, i16* %a, i32 3
+  %addr.b.3 = getelementptr i16, i16* %b, i32 3
+  %ld.a.3 = load i16, i16* %addr.a.3
+  %sext.a.3 = sext i16 %ld.a.3 to i32
+  %ld.b.3 = load i16, i16* %addr.b.3
+  %sext.b.3 = sext i16 %ld.b.3 to i32
+  %mul.3 = mul i32 %sext.a.3, %sext.b.3
+  %sext.mul.3 = sext i32 %mul.3 to i64
+  %add.2 = add i64 %sext.mul.2, %sext.mul.3
+  %add.3 = add i64 %add.1, %add.2
+  ret i64 %add.3
+}
+
+; CHECK-LABEL: sext_acc_3
+; CHECK: [[CAST_A:%[^ ]+]] = bitcast i16* %a to i32*
+; CHECK: [[A:%[^ ]+]] = load i32, i32* [[CAST_A]]
+; CHECK: [[CAST_B:%[^ ]+]] = bitcast i16* %b to i32*
+; CHECK: [[B:%[^ ]+]] = load i32, i32* [[CAST_B]]
+; CHECK: [[CAST_A_2:%[^ ]+]] = bitcast i16* %addr.a.2 to i32*
+; CHECK: [[A_2:%[^ ]+]] = load i32, i32* %4
+; CHECK: [[CAST_B_2:%[^ ]+]] = bitcast i16* %addr.b.2 to i32*
+; CHECK: [[B_2:%[^ ]+]] = load i32, i32* %6
+; CHECK: [[ACC:%[^ ]+]] = sext i32 %acc to i64
+; CHECK: [[SMLALD:%[^ ]+]] = call i64 @llvm.arm.smlald(i32 [[A]], i32 [[B]], i64 [[ACC]])
+; CHECK: call i64 @llvm.arm.smlald(i32 [[A_2]], i32 [[B_2]], i64 [[SMLALD]])
+define i64 @sext_acc_3(i16* %a, i16* %b, i32 %acc) {
+entry:
+  %ld.a.0 = load i16, i16* %a
+  %sext.a.0 = sext i16 %ld.a.0 to i32
+  %ld.b.0 = load i16, i16* %b
+  %sext.b.0 = sext i16 %ld.b.0 to i32
+  %mul.0 = mul i32 %sext.a.0, %sext.b.0
+  %addr.a.1 = getelementptr i16, i16* %a, i32 1
+  %addr.b.1 = getelementptr i16, i16* %b, i32 1
+  %ld.a.1 = load i16, i16* %addr.a.1
+  %sext.a.1 = sext i16 %ld.a.1 to i32
+  %ld.b.1 = load i16, i16* %addr.b.1
+  %sext.b.1 = sext i16 %ld.b.1 to i32
+  %mul.1 = mul i32 %sext.a.1, %sext.b.1
+  %sext.mul.0 = sext i32 %mul.0 to i64
+  %sext.mul.1 = sext i32 %mul.1 to i64
+  %add = add i64 %sext.mul.0, %sext.mul.1
+  %addr.a.2 = getelementptr i16, i16* %a, i32 2
+  %addr.b.2 = getelementptr i16, i16* %b, i32 2
+  %ld.a.2 = load i16, i16* %addr.a.2
+  %sext.a.2 = sext i16 %ld.a.2 to i32
+  %ld.b.2 = load i16, i16* %addr.b.2
+  %sext.b.2 = sext i16 %ld.b.2 to i32
+  %mul.2 = mul i32 %sext.a.2, %sext.b.2
+  %sext.mul.2 = sext i32 %mul.2 to i64
+  %addr.a.3 = getelementptr i16, i16* %a, i32 3
+  %addr.b.3 = getelementptr i16, i16* %b, i32 3
+  %ld.a.3 = load i16, i16* %addr.a.3
+  %sext.a.3 = sext i16 %ld.a.3 to i32
+  %ld.b.3 = load i16, i16* %addr.b.3
+  %sext.b.3 = sext i16 %ld.b.3 to i32
+  %mul.3 = mul i32 %sext.a.3, %sext.b.3
+  %sext.mul.3 = sext i32 %mul.3 to i64
+  %add.1 = add i64 %sext.mul.2, %sext.mul.3
+  %add.2 = add i64 %add, %add.1
+  %sext.acc = sext i32 %acc to i64
+  %add.3 = add i64 %add.2, %sext.acc
+  ret i64 %add.3
+}
+
+; CHECK-LABEL: sext_acc_4
+; CHECK: [[CAST_A:%[^ ]+]] = bitcast i16* %a to i32*
+; CHECK: [[A:%[^ ]+]] = load i32, i32* [[CAST_A]]
+; CHECK: [[CAST_B:%[^ ]+]] = bitcast i16* %b to i32*
+; CHECK: [[B:%[^ ]+]] = load i32, i32* [[CAST_B]]
+; CHECK: [[CAST_A_2:%[^ ]+]] = bitcast i16* %addr.a.2 to i32*
+; CHECK: [[A_2:%[^ ]+]] = load i32, i32* %4
+; CHECK: [[CAST_B_2:%[^ ]+]] = bitcast i16* %addr.b.2 to i32*
+; CHECK: [[B_2:%[^ ]+]] = load i32, i32* %6
+; CHECK: [[ACC:%[^ ]+]] = sext i32 %acc to i64
+; CHECK: [[SMLALD:%[^ ]+]] = call i64 @llvm.arm.smlald(i32 [[A]], i32 [[B]], i64 [[ACC]])
+; CHECK: call i64 @llvm.arm.smlald(i32 [[A_2]], i32 [[B_2]], i64 [[SMLALD]])
+define i64 @sext_acc_4(i16* %a, i16* %b, i32 %acc) {
+entry:
+  %ld.a.0 = load i16, i16* %a
+  %sext.a.0 = sext i16 %ld.a.0 to i32
+  %ld.b.0 = load i16, i16* %b
+  %sext.b.0 = sext i16 %ld.b.0 to i32
+  %mul.0 = mul i32 %sext.a.0, %sext.b.0
+  %addr.a.1 = getelementptr i16, i16* %a, i32 1
+  %addr.b.1 = getelementptr i16, i16* %b, i32 1
+  %ld.a.1 = load i16, i16* %addr.a.1
+  %sext.a.1 = sext i16 %ld.a.1 to i32
+  %ld.b.1 = load i16, i16* %addr.b.1
+  %sext.b.1 = sext i16 %ld.b.1 to i32
+  %mul.1 = mul i32 %sext.a.1, %sext.b.1
+  %add = add i32 %mul.0, %mul.1
+  %sext.add = sext i32 %add to i64
+  %addr.a.2 = getelementptr i16, i16* %a, i32 2
+  %addr.b.2 = getelementptr i16, i16* %b, i32 2
+  %ld.a.2 = load i16, i16* %addr.a.2
+  %sext.a.2 = sext i16 %ld.a.2 to i32
+  %ld.b.2 = load i16, i16* %addr.b.2
+  %sext.b.2 = sext i16 %ld.b.2 to i32
+  %mul.2 = mul i32 %sext.a.2, %sext.b.2
+  %sext.mul.2 = sext i32 %mul.2 to i64
+  %addr.a.3 = getelementptr i16, i16* %a, i32 3
+  %addr.b.3 = getelementptr i16, i16* %b, i32 3
+  %ld.a.3 = load i16, i16* %addr.a.3
+  %sext.a.3 = sext i16 %ld.a.3 to i32
+  %ld.b.3 = load i16, i16* %addr.b.3
+  %sext.b.3 = sext i16 %ld.b.3 to i32
+  %mul.3 = mul i32 %sext.a.3, %sext.b.3
+  %sext.mul.3 = sext i32 %mul.3 to i64
+  %sext.acc = sext i32 %acc to i64
+  %add.1 = add i64 %sext.mul.2, %sext.add
+  %add.2 = add i64 %sext.add, %add.1
+  %add.3 = add i64 %add.2, %sext.mul.3
+  %add.4 = add i64 %add.3, %sext.acc
+  ret i64 %add.4
+}