<HTML><BODY><FONT style='white-space:pre-wrap;font-family: Helvetica Neue, Helvetica, Arial, sans-serif;margin: 1em 0;'>Sure, thanks for reminding, will include why next time.<br><br>For this revert, it was breaking several build.<br>See discussion in <a href='https://reviews.llvm.org/D64197'>https://reviews.llvm.org/D64197</a> for more details if interested.<br><br><br><br></FONT><br><br><div class="domino-section" dir="ltr"><div class="domino-section-head"><span class="domino-section-title"><font color="#424282">Philip Reames --- [EXTERNAL] Re: [llvm] r365520 - Revert "[HardwareLoops] NFC - move hardware loop checking code to isHardwareLoopProfitable()" --- </font></span></div><div class="domino-section-body"><br><table width="100%" border="0" cellspacing="0" cellpadding="0"><tr valign="top"><td width="1%" style="width: 96px;"><font size="2" color="#5F5F5F">From:</font></td><td width="100%" style="width: auto;"><font size="2">"Philip Reames" <listmail@philipreames.com></font></td></tr><tr valign="top"><td width="1%" style="width: 96px;"><font size="2" color="#5F5F5F">To:</font></td><td width="100%" style="width: auto;"><font size="2">"Jinsong Ji" <jji@us.ibm.com>, llvm-commits@lists.llvm.org</font></td></tr><tr valign="top"><td width="1%" style="width: 96px;"><font size="2" color="#5F5F5F">Date:</font></td><td width="100%" style="width: auto;"><font size="2">Tue, Jul 9, 2019 6:09 PM</font></td></tr><tr valign="top"><td width="1%" style="width: 96px;"><font size="2" color="#5F5F5F">Subject:</font></td><td width="100%" style="width: auto;"><font size="2">[EXTERNAL] Re: [llvm] r365520 - Revert "[HardwareLoops] NFC - move hardware loop checking code to isHardwareLoopProfitable()"</font></td></tr></table><hr width="100%" size="2" align="left" noshade style="color:#8091A5; "><br></div><html><body><div style="opacity: 0.87;"><pre style="white-space:pre-wrap;"><font color="#000000">When revert, please say why.<br/><br/>Philip<br/><br/>On 7/9/19 10:53 AM, Jinsong Ji via llvm-commits wrote:<br/><!-- -->> Author: jsji<br/><!-- -->> Date: Tue Jul 9 10:53:09 2019<br/><!-- -->> New Revision: 365520<br/><!-- -->><br/><!-- -->> URL: https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D365520-26view-3Drev&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=DvnnfavFQBGT2CDyHzTr_Q&m=5n5rA3XF6uGi268QFmwK9CN1EZ1gNuuz1wxJVEy64qY&s=Oq9deT9bXxH0NHPyOfs3B-rMYCEZ7SBNkpKzJTyV4fI&e= <br/><!-- -->> Log:<br/><!-- -->> Revert "[HardwareLoops] NFC - move hardware loop checking code to isHardwareLoopProfitable()"<br/><!-- -->><br/><!-- -->> This reverts commit d95557306585404893d610784edb3e32f1bfce18.<br/><!-- -->><br/><!-- -->> Modified:<br/><!-- -->> llvm/trunk/include/llvm/Analysis/TargetTransformInfo.h<br/><!-- -->> llvm/trunk/lib/Analysis/TargetTransformInfo.cpp<br/><!-- -->> llvm/trunk/lib/CodeGen/HardwareLoops.cpp<br/><!-- -->><br/><!-- -->> Modified: llvm/trunk/include/llvm/Analysis/TargetTransformInfo.h<br/><!-- -->> URL: https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_include_llvm_Analysis_TargetTransformInfo.h-3Frev-3D365520-26r1-3D365519-26r2-3D365520-26view-3Ddiff&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=DvnnfavFQBGT2CDyHzTr_Q&m=5n5rA3XF6uGi268QFmwK9CN1EZ1gNuuz1wxJVEy64qY&s=vKu87OjQPNP_J6XkL7GufJagShSUznzaSkhiq1j7bLI&e= <br/><!-- -->> ==============================================================================<br/><!-- -->> --- llvm/trunk/include/llvm/Analysis/TargetTransformInfo.h (original)<br/><!-- -->> +++ llvm/trunk/include/llvm/Analysis/TargetTransformInfo.h Tue Jul 9 10:53:09 2019<br/><!-- -->> @@ -99,8 +99,7 @@ struct HardwareLoopInfo {<br/><!-- -->> // produces an i1 to guard the loop entry.<br/><!-- -->> bool isHardwareLoopCandidate(ScalarEvolution &SE, LoopInfo &LI,<br/><!-- -->> DominatorTree &DT, bool ForceNestedLoop = false,<br/><!-- -->> - bool ForceHardwareLoopPHI = false,<br/><!-- -->> - bool ForceGuardLoopEntry = false);<br/><!-- -->> + bool ForceHardwareLoopPHI = false);<br/><!-- -->> bool canAnalyze(LoopInfo &LI);<br/><!-- -->> };<br/><!-- -->> <br/><!-- -->><br/><!-- -->> Modified: llvm/trunk/lib/Analysis/TargetTransformInfo.cpp<br/><!-- -->> URL: https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_Analysis_TargetTransformInfo.cpp-3Frev-3D365520-26r1-3D365519-26r2-3D365520-26view-3Ddiff&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=DvnnfavFQBGT2CDyHzTr_Q&m=5n5rA3XF6uGi268QFmwK9CN1EZ1gNuuz1wxJVEy64qY&s=VYoUKwZ8m-SvEz_DoDtim8yKnQ2CXYt6fJ3G3aRbeL4&e= <br/><!-- -->> ==============================================================================<br/><!-- -->> --- llvm/trunk/lib/Analysis/TargetTransformInfo.cpp (original)<br/><!-- -->> +++ llvm/trunk/lib/Analysis/TargetTransformInfo.cpp Tue Jul 9 10:53:09 2019<br/><!-- -->> @@ -20,9 +20,6 @@<br/><!-- -->> #include "llvm/Support/ErrorHandling.h"<br/><!-- -->> #include "llvm/Analysis/CFG.h"<br/><!-- -->> #include "llvm/Analysis/LoopIterator.h"<br/><!-- -->> -#include "llvm/Transforms/Utils.h"<br/><!-- -->> -#include "llvm/Transforms/Utils/LoopUtils.h"<br/><!-- -->> -#include "llvm/Analysis/ScalarEvolutionExpander.h"<br/><!-- -->> #include <!-- --><utility<!-- -->><br/><!-- -->> <br/><!-- -->> using namespace llvm;<br/><!-- -->> @@ -58,8 +55,7 @@ bool HardwareLoopInfo::canAnalyze(LoopIn<br/><!-- -->> bool HardwareLoopInfo::isHardwareLoopCandidate(ScalarEvolution &SE,<br/><!-- -->> LoopInfo &LI, DominatorTree &DT,<br/><!-- -->> bool ForceNestedLoop,<br/><!-- -->> - bool ForceHardwareLoopPHI,<br/><!-- -->> - bool ForceGuardLoopEntry) {<br/><!-- -->> + bool ForceHardwareLoopPHI) {<br/><!-- -->> SmallVector<!-- --><BasicBlock *, 4<!-- -->> ExitingBlocks;<br/><!-- -->> L-<!-- -->>getExitingBlocks(ExitingBlocks);<br/><!-- -->> <br/><!-- -->> @@ -138,33 +134,6 @@ bool HardwareLoopInfo::isHardwareLoopCan<br/><!-- -->> <br/><!-- -->> if (!ExitBlock)<br/><!-- -->> return false;<br/><!-- -->> -<br/><!-- -->> - BasicBlock *Preheader = L-<!-- -->>getLoopPreheader();<br/><!-- -->> -<br/><!-- -->> - // If we don't have a preheader, then insert one.<br/><!-- -->> - if (!Preheader)<br/><!-- -->> - Preheader = InsertPreheaderForLoop(L, &DT, &LI, nullptr, true);<br/><!-- -->> - if (!Preheader)<br/><!-- -->> - return false;<br/><!-- -->> -<br/><!-- -->> - // Make sure we have a valid Loop Count<br/><!-- -->> - if (!ExitCount-<!-- -->>getType()-<!-- -->>isPointerTy() &<!-- -->&ExitCount-<!-- -->>getType() != CountType)<br/><!-- -->> - ExitCount = SE.getZeroExtendExpr(ExitCount, CountType);<br/><!-- -->> -<br/><!-- -->> - ExitCount = SE.getAddExpr(ExitCount, SE.getOne(CountType));<br/><!-- -->> -<br/><!-- -->> - BasicBlock *BB = L-<!-- -->>getLoopPreheader();<br/><!-- -->> -<br/><!-- -->> - if (ForceGuardLoopEntry &<!-- -->&BB-<!-- -->>getSinglePredecessor() &<!-- -->&<br/><!-- -->> - cast<!-- --><BranchInst<!-- -->>(BB-<!-- -->>getTerminator())-<!-- -->>isUnconditional())<br/><!-- -->> - BB = BB-<!-- -->>getSinglePredecessor();<br/><!-- -->> -<br/><!-- -->> - if (!isSafeToExpandAt(ExitCount, BB-<!-- -->>getTerminator(), SE)) {<br/><!-- -->> - LLVM_DEBUG(dbgs() <!-- --><<!-- -->< "Not a Hardware Loop: unsafe to expand ExitCount "<br/><!-- -->> - <!-- --><<!-- -->< *ExitCount <!-- --><<!-- -->< "\n");<br/><!-- -->> - return false;<br/><!-- -->> - }<br/><!-- -->> -<br/><!-- -->> return true;<br/><!-- -->> }<br/><!-- -->> <br/><!-- -->><br/><!-- -->> Modified: llvm/trunk/lib/CodeGen/HardwareLoops.cpp<br/><!-- -->> URL: https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_CodeGen_HardwareLoops.cpp-3Frev-3D365520-26r1-3D365519-26r2-3D365520-26view-3Ddiff&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=DvnnfavFQBGT2CDyHzTr_Q&m=5n5rA3XF6uGi268QFmwK9CN1EZ1gNuuz1wxJVEy64qY&s=RjYspxycuwL1Xme86I9nA1Q_eT7ObjKQ-JP1mxiowS4&e= <br/><!-- -->> ==============================================================================<br/><!-- -->> --- llvm/trunk/lib/CodeGen/HardwareLoops.cpp (original)<br/><!-- -->> +++ llvm/trunk/lib/CodeGen/HardwareLoops.cpp Tue Jul 9 10:53:09 2019<br/><!-- -->> @@ -15,6 +15,7 @@<br/><!-- -->> ///<br/><!-- -->> //===----------------------------------------------------------------------===//<br/><!-- -->> <br/><!-- -->> +#include "llvm/Pass.h"<br/><!-- -->> #include "llvm/PassRegistry.h"<br/><!-- -->> #include "llvm/PassSupport.h"<br/><!-- -->> #include "llvm/ADT/Statistic.h"<br/><!-- -->> @@ -35,8 +36,10 @@<br/><!-- -->> #include "llvm/IR/Value.h"<br/><!-- -->> #include "llvm/Support/Debug.h"<br/><!-- -->> #include "llvm/Transforms/Scalar.h"<br/><!-- -->> +#include "llvm/Transforms/Utils.h"<br/><!-- -->> #include "llvm/Transforms/Utils/BasicBlockUtils.h"<br/><!-- -->> #include "llvm/Transforms/Utils/Local.h"<br/><!-- -->> +#include "llvm/Transforms/Utils/LoopUtils.h"<br/><!-- -->> <br/><!-- -->> #define DEBUG_TYPE "hardware-loops"<br/><!-- -->> <br/><!-- -->> @@ -109,6 +112,7 @@ namespace {<br/><!-- -->> const DataLayout *DL = nullptr;<br/><!-- -->> const TargetTransformInfo *TTI = nullptr;<br/><!-- -->> DominatorTree *DT = nullptr;<br/><!-- -->> + bool PreserveLCSSA = false;<br/><!-- -->> AssumptionCache *AC = nullptr;<br/><!-- -->> TargetLibraryInfo *LibInfo = nullptr;<br/><!-- -->> Module *M = nullptr;<br/><!-- -->> @@ -180,6 +184,7 @@ bool HardwareLoops::runOnFunction(Functi<br/><!-- -->> DL = &F.getParent()-<!-- -->>getDataLayout();<br/><!-- -->> auto *TLIP = getAnalysisIfAvailable<!-- --><TargetLibraryInfoWrapperPass<!-- -->>();<br/><!-- -->> LibInfo = TLIP ? &TLIP-<!-- -->>getTLI() : nullptr;<br/><!-- -->> + PreserveLCSSA = mustPreserveAnalysisID(LCSSAID);<br/><!-- -->> AC = &getAnalysis<!-- --><AssumptionCacheTracker<!-- -->>().getAssumptionCache(F);<br/><!-- -->> M = F.getParent();<br/><!-- -->> <br/><!-- -->> @@ -225,19 +230,25 @@ bool HardwareLoops::TryConvertLoop(Loop<br/><!-- -->> <br/><!-- -->> bool HardwareLoops::TryConvertLoop(HardwareLoopInfo &HWLoopInfo) {<br/><!-- -->> <br/><!-- -->> - LLVM_DEBUG(dbgs() <!-- --><<!-- -->< "HWLoops: Try to convert profitable loop: "<br/><!-- -->> - <!-- --><<!-- -->< *HWLoopInfo.L);<br/><!-- -->> + Loop *L = HWLoopInfo.L;<br/><!-- -->> + LLVM_DEBUG(dbgs() <!-- --><<!-- -->< "HWLoops: Try to convert profitable loop: " <!-- --><<!-- -->< *L);<br/><!-- -->> <br/><!-- -->> if (!HWLoopInfo.isHardwareLoopCandidate(*SE, *LI, *DT, ForceNestedLoop,<br/><!-- -->> - ForceHardwareLoopPHI,<br/><!-- -->> - ForceGuardLoopEntry))<br/><!-- -->> + ForceHardwareLoopPHI))<br/><!-- -->> return false;<br/><!-- -->> <br/><!-- -->> assert(<br/><!-- -->> (HWLoopInfo.ExitBlock &<!-- -->&HWLoopInfo.ExitBranch &<!-- -->&HWLoopInfo.ExitCount) &<!-- -->&<br/><!-- -->> "Hardware Loop must have set exit info.");<br/><!-- -->> <br/><!-- -->> - // Now start to converting...<br/><!-- -->> + BasicBlock *Preheader = L-<!-- -->>getLoopPreheader();<br/><!-- -->> +<br/><!-- -->> + // If we don't have a preheader, then insert one.<br/><!-- -->> + if (!Preheader)<br/><!-- -->> + Preheader = InsertPreheaderForLoop(L, DT, LI, nullptr, PreserveLCSSA);<br/><!-- -->> + if (!Preheader)<br/><!-- -->> + return false;<br/><!-- -->> +<br/><!-- -->> HardwareLoop HWLoop(HWLoopInfo, *SE, *DL);<br/><!-- -->> HWLoop.Create();<br/><!-- -->> ++NumHWLoops;<br/><!-- -->> @@ -246,10 +257,10 @@ bool HardwareLoops::TryConvertLoop(Hardw<br/><!-- -->> <br/><!-- -->> void HardwareLoop::Create() {<br/><!-- -->> LLVM_DEBUG(dbgs() <!-- --><<!-- -->< "HWLoops: Converting loop..\n");<br/><!-- -->> -<br/><!-- -->> +<br/><!-- -->> Value *LoopCountInit = InitLoopCount();<br/><!-- -->> -<br/><!-- -->> - assert(LoopCountInit &<!-- -->&"Hardware Loop must have a loop count");<br/><!-- -->> + if (!LoopCountInit)<br/><!-- -->> + return;<br/><!-- -->> <br/><!-- -->> InsertIterationSetup(LoopCountInit);<br/><!-- -->> <br/><!-- -->> @@ -309,22 +320,32 @@ Value *HardwareLoop::InitLoopCount() {<br/><!-- -->> // loop counter and tests that is not zero?<br/><!-- -->> <br/><!-- -->> SCEVExpander SCEVE(SE, DL, "loopcnt");<br/><!-- -->> + if (!ExitCount-<!-- -->>getType()-<!-- -->>isPointerTy() &<!-- -->&<br/><!-- -->> + ExitCount-<!-- -->>getType() != CountType)<br/><!-- -->> + ExitCount = SE.getZeroExtendExpr(ExitCount, CountType);<br/><!-- -->> +<br/><!-- -->> + ExitCount = SE.getAddExpr(ExitCount, SE.getOne(CountType));<br/><!-- -->> <br/><!-- -->> // If we're trying to use the 'test and set' form of the intrinsic, we need<br/><!-- -->> // to replace a conditional branch that is controlling entry to the loop. It<br/><!-- -->> // is likely (guaranteed?) that the preheader has an unconditional branch to<br/><!-- -->> // the loop header, so also check if it has a single predecessor.<br/><!-- -->> if (SE.isLoopEntryGuardedByCond(L, ICmpInst::ICMP_NE, ExitCount,<br/><!-- -->> - SE.getZero(ExitCount-<!-- -->>getType())))<br/><!-- -->> + SE.getZero(ExitCount-<!-- -->>getType()))) {<br/><!-- -->> + LLVM_DEBUG(dbgs() <!-- --><<!-- -->< " - Attempting to use test.set counter.\n");<br/><!-- -->> UseLoopGuard |= ForceGuardLoopEntry;<br/><!-- -->> - else<br/><!-- -->> + } else<br/><!-- -->> UseLoopGuard = false;<br/><!-- -->> <br/><!-- -->> BasicBlock *BB = L-<!-- -->>getLoopPreheader();<br/><!-- -->> if (UseLoopGuard &<!-- -->&BB-<!-- -->>getSinglePredecessor() &<!-- -->&<br/><!-- -->> - cast<!-- --><BranchInst<!-- -->>(BB-<!-- -->>getTerminator())-<!-- -->>isUnconditional()) {<br/><!-- -->> - LLVM_DEBUG(dbgs() <!-- --><<!-- -->< " - Attempting to use test.set counter.\n");<br/><!-- -->> + cast<!-- --><BranchInst<!-- -->>(BB-<!-- -->>getTerminator())-<!-- -->>isUnconditional())<br/><!-- -->> BB = BB-<!-- -->>getSinglePredecessor();<br/><!-- -->> +<br/><!-- -->> + if (!isSafeToExpandAt(ExitCount, BB-<!-- -->>getTerminator(), SE)) {<br/><!-- -->> + LLVM_DEBUG(dbgs() <!-- --><<!-- -->< "- Bailing, unsafe to expand ExitCount "<br/><!-- -->> + <!-- --><<!-- -->< *ExitCount <!-- --><<!-- -->< "\n");<br/><!-- -->> + return nullptr;<br/><!-- -->> }<br/><!-- -->> <br/><!-- -->> Value *Count = SCEVE.expandCodeFor(ExitCount, CountType,<br/><!-- -->><br/><!-- -->><br/><!-- -->> _______________________________________________<br/><!-- -->> llvm-commits mailing list<br/><!-- -->> llvm-commits@lists.llvm.org<br/><!-- -->> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Dcommits&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=DvnnfavFQBGT2CDyHzTr_Q&m=5n5rA3XF6uGi268QFmwK9CN1EZ1gNuuz1wxJVEy64qY&s=_3ZaFmu5fpyyYWG5LZ_-V5_iljWLVpi_YcVZdKzRr3k&e= <br/><br/></font></pre></div><BR>
</body></html></BODY></HTML>