<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Hi Eli,</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
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.</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Thanks,</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
sam</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div id="Signature">
<div id="divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:rgb(0,0,0); background-color:rgb(255,255,255); font-family:Calibri,Arial,Helvetica,sans-serif,EmojiFont,"Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols,EmojiFont,"Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols">
<p style="margin-top: 0px; margin-bottom: 0px;"></p>
<p style="margin-top: 0px; margin-bottom: 0px;font-family:"Times New Roman""><span style="font-family:Calibri,Helvetica,sans-serif">Sam Parker</span></p>
<span style="font-family:Calibri,Helvetica,sans-serif"></span>
<p style="margin-top: 0px; margin-bottom: 0px;font-family:"Times New Roman""><span style="font-family:Calibri,Helvetica,sans-serif">Compilation Tools Engineer | Arm</span></p>
<span style="font-family:Calibri,Helvetica,sans-serif"></span>
<p style="margin-top: 0px; margin-bottom: 0px;font-family:"Times New Roman""><span style="font-family:Calibri,Helvetica,sans-serif">. . . . . . . . . . . . . . . . . . . . . . . . . . .</span></p>
<span style="font-family:Calibri,Helvetica,sans-serif"></span>
<p style="margin-top: 0px; margin-bottom: 0px;font-family:"Times New Roman""><span style="font-family:Calibri,Helvetica,sans-serif">Arm.com</span></p>
<p style="margin-top: 0px; margin-bottom: 0px;"></p>
</div>
</div>
<div id="appendonsend"></div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Eli Friedman <efriedma@quicinc.com><br>
<b>Sent:</b> 02 August 2019 18:59<br>
<b>To:</b> Sam Parker <Sam.Parker@arm.com><br>
<b>Cc:</b> llvm-commits <llvm-commits@lists.llvm.org><br>
<b>Subject:</b> RE: [llvm] r367522 - [NFC][ARM][ParallelDSP] Getters and renaming</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">Hi Sam,<br>
<br>
This, or one of the other changes to ARMParallelDSP between r367504 and r367642, seems to have broken Android builds. See
<a href="http://lab.llvm.org:8011/builders/aosp-O3-polly-before-vectorizer-unprofitable/builds/973/steps/build-aosp/logs/stdio">
http://lab.llvm.org:8011/builders/aosp-O3-polly-before-vectorizer-unprofitable/builds/973/steps/build-aosp/logs/stdio</a><br>
<br>
Can you tell what's going on from the crash message, or do you want me to reduce a testcase?<br>
<br>
-Eli<br>
<br>
-----Original Message-----<br>
From: llvm-commits <llvm-commits-bounces@lists.llvm.org> On Behalf Of Sam Parker via llvm-commits<br>
Sent: Thursday, August 1, 2019 1:18 AM<br>
To: llvm-commits@lists.llvm.org<br>
Subject: [EXT] [llvm] r367522 - [NFC][ARM][ParallelDSP] Getters and renaming<br>
<br>
Author: sam_parker<br>
Date: Thu Aug 1 01:17:51 2019<br>
New Revision: 367522<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=367522&view=rev">http://llvm.org/viewvc/llvm-project?rev=367522&view=rev</a><br>
Log:<br>
[NFC][ARM][ParallelDSP] Getters and renaming<br>
<br>
Add a couple of getters for Reduction and do some renaming of<br>
variables around CreateSMLAD for clarity.<br>
<br>
Modified:<br>
llvm/trunk/lib/Target/ARM/ARMParallelDSP.cpp<br>
<br>
Modified: llvm/trunk/lib/Target/ARM/ARMParallelDSP.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMParallelDSP.cpp?rev=367522&r1=367521&r2=367522&view=diff">
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMParallelDSP.cpp?rev=367522&r1=367521&r2=367522&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/ARM/ARMParallelDSP.cpp (original)<br>
+++ llvm/trunk/lib/Target/ARM/ARMParallelDSP.cpp Thu Aug 1 01:17:51 2019<br>
@@ -70,6 +70,10 @@ namespace {<br>
bool HasTwoLoadInputs() const {<br>
return isa<LoadInst>(LHS) && isa<LoadInst>(RHS);<br>
}<br>
+<br>
+ LoadInst *getBaseLoad() const {<br>
+ return cast<LoadInst>(LHS);<br>
+ }<br>
};<br>
<br>
/// Represent a sequence of multiply-accumulate operations with the aim to<br>
@@ -118,6 +122,8 @@ namespace {<br>
/// Return the add instruction which is the root of the reduction.<br>
Instruction *getRoot() { return Root; }<br>
<br>
+ bool is64Bit() const { return Root->getType()->isIntegerTy(64); }<br>
+<br>
/// Return the incoming value to be accumulated. This maybe null.<br>
Value *getAccumulator() { return Acc; }<br>
<br>
@@ -594,16 +600,10 @@ bool ARMParallelDSP::CreateParallelPairs<br>
<br>
void ARMParallelDSP::InsertParallelMACs(Reduction &R) {<br>
<br>
- auto CreateSMLADCall = [&](SmallVectorImpl<LoadInst*> &VecLd0,<br>
- SmallVectorImpl<LoadInst*> &VecLd1,<br>
- Value *Acc, bool Exchange,<br>
- Instruction *InsertAfter) {<br>
+ auto CreateSMLAD = [&](LoadInst* WideLd0, LoadInst *WideLd1,<br>
+ Value *Acc, bool Exchange,<br>
+ Instruction *InsertAfter) {<br>
// Replace the reduction chain with an intrinsic call<br>
- IntegerType *Ty = IntegerType::get(M->getContext(), 32);<br>
- LoadInst *WideLd0 = WideLoads.count(VecLd0[0]) ?<br>
- WideLoads[VecLd0[0]]->getLoad() : CreateWideLoad(VecLd0, Ty);<br>
- LoadInst *WideLd1 = WideLoads.count(VecLd1[0]) ?<br>
- WideLoads[VecLd1[0]]->getLoad() : CreateWideLoad(VecLd1, Ty);<br>
<br>
Value* Args[] = { WideLd0, WideLd1, Acc };<br>
Function *SMLAD = nullptr;<br>
@@ -628,17 +628,23 @@ void ARMParallelDSP::InsertParallelMACs(<br>
if (!Acc)<br>
Acc = ConstantInt::get(IntegerType::get(M->getContext(), 32), 0);<br>
<br>
+ IntegerType *Ty = IntegerType::get(M->getContext(), 32);<br>
LLVM_DEBUG(dbgs() << "Root: " << *InsertAfter << "\n"<br>
<< "Acc: " << *Acc << "\n");<br>
for (auto &Pair : R.getMulPairs()) {<br>
- MulCandidate *PMul0 = Pair.first;<br>
- MulCandidate *PMul1 = Pair.second;<br>
+ MulCandidate *LHSMul = Pair.first;<br>
+ MulCandidate *RHSMul = Pair.second;<br>
LLVM_DEBUG(dbgs() << "Muls:\n"<br>
- << "- " << *PMul0->Root << "\n"<br>
- << "- " << *PMul1->Root << "\n");<br>
+ << "- " << *LHSMul->Root << "\n"<br>
+ << "- " << *RHSMul->Root << "\n");<br>
+ LoadInst *BaseLHS = LHSMul->getBaseLoad();<br>
+ LoadInst *BaseRHS = RHSMul->getBaseLoad();<br>
+ LoadInst *WideLHS = WideLoads.count(BaseLHS) ?<br>
+ WideLoads[BaseLHS]->getLoad() : CreateWideLoad(LHSMul->VecLd, Ty);<br>
+ LoadInst *WideRHS = WideLoads.count(BaseRHS) ?<br>
+ WideLoads[BaseRHS]->getLoad() : CreateWideLoad(RHSMul->VecLd, Ty);<br>
<br>
- Acc = CreateSMLADCall(PMul0->VecLd, PMul1->VecLd, Acc, PMul1->Exchange,<br>
- InsertAfter);<br>
+ Acc = CreateSMLAD(WideLHS, WideRHS, Acc, RHSMul->Exchange, InsertAfter);<br>
InsertAfter = cast<Instruction>(Acc);<br>
}<br>
R.UpdateRoot(cast<Instruction>(Acc));<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
llvm-commits@lists.llvm.org<br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</div>
</span></font></div>
</body>
</html>