[llvm] r367522 - [NFC][ARM][ParallelDSP] Getters and renaming

[Sam Parker via llvm-commits]

Hi Eli,

So much for NFC, sorry! Looks like 'getBaseLoad' should be returning VecLd[0], not LHS. I'll put together a test case and get it up for review later today.

Thanks,
sam


Sam Parker

Compilation Tools Engineer | Arm

. . . . . . . . . . . . . . . . . . . . . . . . . . .

Arm.com

________________________________
From: Eli Friedman <efriedma at quicinc.com>
Sent: 02 August 2019 18:59
To: Sam Parker <Sam.Parker at arm.com>
Cc: llvm-commits <llvm-commits at lists.llvm.org>
Subject: RE: [llvm] r367522 - [NFC][ARM][ParallelDSP] Getters and renaming

Hi Sam,

This, or one of the other changes to ARMParallelDSP between r367504 and r367642, seems to have broken Android builds.  See http://lab.llvm.org:8011/builders/aosp-O3-polly-before-vectorizer-unprofitable/builds/973/steps/build-aosp/logs/stdio

Can you tell what's going on from the crash message, or do you want me to reduce a testcase?

-Eli

-----Original Message-----
From: llvm-commits <llvm-commits-bounces at lists.llvm.org> On Behalf Of Sam Parker via llvm-commits
Sent: Thursday, August 1, 2019 1:18 AM
To: llvm-commits at lists.llvm.org
Subject: [EXT] [llvm] r367522 - [NFC][ARM][ParallelDSP] Getters and renaming

Author: sam_parker
Date: Thu Aug  1 01:17:51 2019
New Revision: 367522

URL: http://llvm.org/viewvc/llvm-project?rev=367522&view=rev
Log:
[NFC][ARM][ParallelDSP] Getters and renaming

Add a couple of getters for Reduction and do some renaming of
variables around CreateSMLAD for clarity.

Modified:
    llvm/trunk/lib/Target/ARM/ARMParallelDSP.cpp

Modified: llvm/trunk/lib/Target/ARM/ARMParallelDSP.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMParallelDSP.cpp?rev=367522&r1=367521&r2=367522&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMParallelDSP.cpp (original)
+++ llvm/trunk/lib/Target/ARM/ARMParallelDSP.cpp Thu Aug  1 01:17:51 2019
@@ -70,6 +70,10 @@ namespace {
     bool HasTwoLoadInputs() const {
       return isa<LoadInst>(LHS) && isa<LoadInst>(RHS);
     }
+
+    LoadInst *getBaseLoad() const {
+      return cast<LoadInst>(LHS);
+    }
   };

   /// Represent a sequence of multiply-accumulate operations with the aim to
@@ -118,6 +122,8 @@ namespace {
     /// Return the add instruction which is the root of the reduction.
     Instruction *getRoot() { return Root; }

+    bool is64Bit() const { return Root->getType()->isIntegerTy(64); }
+
     /// Return the incoming value to be accumulated. This maybe null.
     Value *getAccumulator() { return Acc; }

@@ -594,16 +600,10 @@ bool ARMParallelDSP::CreateParallelPairs

 void ARMParallelDSP::InsertParallelMACs(Reduction &R) {

-  auto CreateSMLADCall = [&](SmallVectorImpl<LoadInst*> &VecLd0,
-                             SmallVectorImpl<LoadInst*> &VecLd1,
-                             Value *Acc, bool Exchange,
-                             Instruction *InsertAfter) {
+  auto CreateSMLAD = [&](LoadInst* WideLd0, LoadInst *WideLd1,
+                         Value *Acc, bool Exchange,
+                         Instruction *InsertAfter) {
     // Replace the reduction chain with an intrinsic call
-    IntegerType *Ty = IntegerType::get(M->getContext(), 32);
-    LoadInst *WideLd0 = WideLoads.count(VecLd0[0]) ?
-      WideLoads[VecLd0[0]]->getLoad() : CreateWideLoad(VecLd0, Ty);
-    LoadInst *WideLd1 = WideLoads.count(VecLd1[0]) ?
-      WideLoads[VecLd1[0]]->getLoad() : CreateWideLoad(VecLd1, Ty);

     Value* Args[] = { WideLd0, WideLd1, Acc };
     Function *SMLAD = nullptr;
@@ -628,17 +628,23 @@ void ARMParallelDSP::InsertParallelMACs(
   if (!Acc)
     Acc = ConstantInt::get(IntegerType::get(M->getContext(), 32), 0);

+  IntegerType *Ty = IntegerType::get(M->getContext(), 32);
   LLVM_DEBUG(dbgs() << "Root: " << *InsertAfter << "\n"
              << "Acc: " << *Acc << "\n");
   for (auto &Pair : R.getMulPairs()) {
-    MulCandidate *PMul0 = Pair.first;
-    MulCandidate *PMul1 = Pair.second;
+    MulCandidate *LHSMul = Pair.first;
+    MulCandidate *RHSMul = Pair.second;
     LLVM_DEBUG(dbgs() << "Muls:\n"
-               << "- " << *PMul0->Root << "\n"
-               << "- " << *PMul1->Root << "\n");
+               << "- " << *LHSMul->Root << "\n"
+               << "- " << *RHSMul->Root << "\n");
+    LoadInst *BaseLHS = LHSMul->getBaseLoad();
+    LoadInst *BaseRHS = RHSMul->getBaseLoad();
+    LoadInst *WideLHS = WideLoads.count(BaseLHS) ?
+      WideLoads[BaseLHS]->getLoad() : CreateWideLoad(LHSMul->VecLd, Ty);
+    LoadInst *WideRHS = WideLoads.count(BaseRHS) ?
+      WideLoads[BaseRHS]->getLoad() : CreateWideLoad(RHSMul->VecLd, Ty);

-    Acc = CreateSMLADCall(PMul0->VecLd, PMul1->VecLd, Acc, PMul1->Exchange,
-                          InsertAfter);
+    Acc = CreateSMLAD(WideLHS, WideRHS, Acc, RHSMul->Exchange, InsertAfter);
     InsertAfter = cast<Instruction>(Acc);
   }
   R.UpdateRoot(cast<Instruction>(Acc));


_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190805/4d2362a8/attachment.html>