[llvm] r315826 - [TableGen] Avoid unnecessary std::string creations
Daniel Sanders via llvm-commits
llvm-commits at lists.llvm.org
Sun Oct 15 12:04:22 PDT 2017
I've committed a patch that makes the predicates (particularly the ones that call getPredCode()) much cheaper in r315872.
Here's the updated stats for AArch64:
AArch64GenGlobalISelEmitter.inc
0 calls to getPredCode()
0 calls to getImmCode(). (GlobalISelEmitter doesn't use getImmCode() when printing the immediates predicates)
AArch64GenDAGISelEmitter.inc
450 calls to getPredCode()
547 calls to getImmCode()
The numbers for SelectionDAG still seem a bit high but they're much more reasonable now.
Does that help with the windows builds?
> On 15 Oct 2017, at 00:16, Daniel Sanders via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
>
>
>> On 14 Oct 2017, at 23:13, Simon Pilgrim <llvm-dev at redking.me.uk <mailto:llvm-dev at redking.me.uk>> wrote:
>>
>> Hi Daniel,
>>
>> I don’t have any tablegen changes planned right now, and will put up up for review anything that does arise.
>
> Thanks
>
>> My main concern is that those std::string creations are having a noticeable effect on windows debug tablegen performance. Please continue to keep StringRef wherever possible even if it means discrepancies in the API, and keep an eye out for build time regressions.
>
> This is surprising since I'd have expected them to be called a small number of times (<100) to emit code but I've just tried some quick instrumentation and there's over 7,000 calls to getImmCode() in the generation of AArch64GenGlobalISelEmitter.inc alone and AArch64GenDAGISel.inc is showing over 10,000.
>
> Here's the stats for AArch64:
> AArch64GenGlobalISelEmitter.inc
> 12,258 calls to getPredCode()
> 7,007 calls to getImmCode()
> 8,900 calls to isAlwaysTrue() which calls both getPredCode() and getImmCode() solely to check they're empty
> 2,461 calls to isImmediatePredicate() which calls getImmCode() solely to check it's empty
> AArch64GenDAGISelEmitter.inc
> 19,564 calls to getPredCode()
> 10,398 calls to getImmCode()
> 6,500 calls to isAlwaysTrue() which calls both getPredCode() and getImmCode() solely to check they're empty
> 0 calls to isImmediatePredicate()
>
> As you can see, many of these calls are solely to test the returned string is empty so I suspect a significant amount of it may be overly expensive predicates. Even accounting for that though, there's still an excessive number of calls. I'll look into this in the morning while I'm landing the last couple patches.
>
>> Cheers, Simon.
>>
>>> On 15 Oct 2017, at 02:23, Daniel Sanders <daniel_l_sanders at apple.com <mailto:daniel_l_sanders at apple.com>> wrote:
>>>
>>> Hi Simon,
>>>
>>> We seem to be conflicting with each others commits at the moment. I'm currently trying to land several patches to GlobalISel emitter and some of them make a few of these (currently) unnecessary std::string creations necessary. I need to partially revert this patch so that getPredCode() returns a std::string. I'm also going to revert the getImmCode() change because I think getPredCode() and getImmCode() should have the same interface. The getImmType() and getImmTypeIdentifier() changes look ok to me.
>>>
>>> Do you have any other cleanups affecting GlobalISelEmitter.cpp planned? If so, we should probably discuss them to see if our patches conflict.
>>>
>>>> On 14 Oct 2017, at 14:27, Simon Pilgrim via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>>>>
>>>> Author: rksimon
>>>> Date: Sat Oct 14 14:27:53 2017
>>>> New Revision: 315826
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=315826&view=rev <http://llvm.org/viewvc/llvm-project?rev=315826&view=rev>
>>>> Log:
>>>> [TableGen] Avoid unnecessary std::string creations
>>>>
>>>> Avoid unnecessary std::string creations in the TreePredicateFn getters.
>>>>
>>>> Modified:
>>>> llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp
>>>> llvm/trunk/utils/TableGen/CodeGenDAGPatterns.h
>>>> llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp
>>>>
>>>> Modified: llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp?rev=315826&r1=315825&r2=315826&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp?rev=315826&r1=315825&r2=315826&view=diff>
>>>> ==============================================================================
>>>> --- llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp (original)
>>>> +++ llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp Sat Oct 14 14:27:53 2017
>>>> @@ -854,11 +854,11 @@ TreePredicateFn::TreePredicateFn(TreePat
>>>> ".td file corrupt: can't have a node predicate *and* an imm predicate");
>>>> }
>>>>
>>>> -std::string TreePredicateFn::getPredCode() const {
>>>> +StringRef TreePredicateFn::getPredCode() const {
>>>> return PatFragRec->getRecord()->getValueAsString("PredicateCode");
>>>> }
>>>>
>>>> -std::string TreePredicateFn::getImmCode() const {
>>>> +StringRef TreePredicateFn::getImmCode() const {
>>>> return PatFragRec->getRecord()->getValueAsString("ImmediateCode");
>>>> }
>>>>
>>>> @@ -873,7 +873,7 @@ bool TreePredicateFn::immCodeUsesAPFloat
>>>> Unset);
>>>> }
>>>>
>>>> -std::string TreePredicateFn::getImmType() const {
>>>> +StringRef TreePredicateFn::getImmType() const {
>>>> if (immCodeUsesAPInt())
>>>> return "const APInt &";
>>>> if (immCodeUsesAPFloat())
>>>> @@ -881,7 +881,7 @@ std::string TreePredicateFn::getImmType(
>>>> return "int64_t";
>>>> }
>>>>
>>>> -std::string TreePredicateFn::getImmTypeIdentifier() const {
>>>> +StringRef TreePredicateFn::getImmTypeIdentifier() const {
>>>> if (immCodeUsesAPInt())
>>>> return "APInt";
>>>> else if (immCodeUsesAPFloat())
>>>> @@ -906,21 +906,21 @@ std::string TreePredicateFn::getFnName()
>>>> /// appropriate.
>>>> std::string TreePredicateFn::getCodeToRunOnSDNode() const {
>>>> // Handle immediate predicates first.
>>>> - std::string ImmCode = getImmCode();
>>>> + StringRef ImmCode = getImmCode();
>>>> if (!ImmCode.empty()) {
>>>> - std::string Result = " " + getImmType() + " Imm = ";
>>>> + std::string Result = " " + getImmType().str() + " Imm = ";
>>>> if (immCodeUsesAPFloat())
>>>> Result += "cast<ConstantFPSDNode>(Node)->getValueAPF();\n";
>>>> else if (immCodeUsesAPInt())
>>>> Result += "cast<ConstantSDNode>(Node)->getAPIntValue();\n";
>>>> else
>>>> Result += "cast<ConstantSDNode>(Node)->getSExtValue();\n";
>>>> - return Result + ImmCode;
>>>> + return Result + ImmCode.str();
>>>> }
>>>>
>>>> // Handle arbitrary node predicates.
>>>> assert(!getPredCode().empty() && "Don't have any predicate code!");
>>>> - std::string ClassName;
>>>> + StringRef ClassName;
>>>> if (PatFragRec->getOnlyTree()->isLeaf())
>>>> ClassName = "SDNode";
>>>> else {
>>>> @@ -931,9 +931,9 @@ std::string TreePredicateFn::getCodeToRu
>>>> if (ClassName == "SDNode")
>>>> Result = " SDNode *N = Node;\n";
>>>> else
>>>> - Result = " auto *N = cast<" + ClassName + ">(Node);\n";
>>>> + Result = " auto *N = cast<" + ClassName.str() + ">(Node);\n";
>>>>
>>>> - return Result + getPredCode();
>>>> + return Result + getPredCode().str();
>>>> }
>>>>
>>>> //===----------------------------------------------------------------------===//
>>>>
>>>> Modified: llvm/trunk/utils/TableGen/CodeGenDAGPatterns.h
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/CodeGenDAGPatterns.h?rev=315826&r1=315825&r2=315826&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/CodeGenDAGPatterns.h?rev=315826&r1=315825&r2=315826&view=diff>
>>>> ==============================================================================
>>>> --- llvm/trunk/utils/TableGen/CodeGenDAGPatterns.h (original)
>>>> +++ llvm/trunk/utils/TableGen/CodeGenDAGPatterns.h Sat Oct 14 14:27:53 2017
>>>> @@ -452,13 +452,12 @@ public:
>>>> /// getImmediatePredicateCode - Return the code that evaluates this pattern if
>>>> /// this is an immediate predicate. It is an error to call this on a
>>>> /// non-immediate pattern.
>>>> - std::string getImmediatePredicateCode() const {
>>>> - std::string Result = getImmCode();
>>>> + StringRef getImmediatePredicateCode() const {
>>>> + StringRef Result = getImmCode();
>>>> assert(!Result.empty() && "Isn't an immediate pattern!");
>>>> return Result;
>>>> }
>>>>
>>>> -
>>>> bool operator==(const TreePredicateFn &RHS) const {
>>>> return PatFragRec == RHS.PatFragRec;
>>>> }
>>>> @@ -476,15 +475,15 @@ public:
>>>> std::string getCodeToRunOnSDNode() const;
>>>>
>>>> /// Get the data type of the argument to getImmediatePredicateCode().
>>>> - std::string getImmType() const;
>>>> + StringRef getImmType() const;
>>>>
>>>> /// Get a string that describes the type returned by getImmType() but is
>>>> /// usable as part of an identifier.
>>>> - std::string getImmTypeIdentifier() const;
>>>> + StringRef getImmTypeIdentifier() const;
>>>>
>>>> private:
>>>> - std::string getPredCode() const;
>>>> - std::string getImmCode() const;
>>>> + StringRef getPredCode() const;
>>>> + StringRef getImmCode() const;
>>>> bool immCodeUsesAPInt() const;
>>>> bool immCodeUsesAPFloat() const;
>>>> };
>>>>
>>>> Modified: llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp?rev=315826&r1=315825&r2=315826&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp?rev=315826&r1=315825&r2=315826&view=diff>
>>>> ==============================================================================
>>>> --- llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp (original)
>>>> +++ llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp Sat Oct 14 14:27:53 2017
>>>> @@ -68,13 +68,13 @@ namespace {
>>>>
>>>> /// Get the name of the enum value used to number the predicate function.
>>>> std::string getEnumNameForPredicate(const TreePredicateFn &Predicate) {
>>>> - return "GIPFP_" + Predicate.getImmTypeIdentifier() + "_" +
>>>> + return "GIPFP_" + Predicate.getImmTypeIdentifier().str() + "_" +
>>>> Predicate.getFnName();
>>>> }
>>>>
>>>> /// Get the opcode used to check this predicate.
>>>> std::string getMatchOpcodeForPredicate(const TreePredicateFn &Predicate) {
>>>> - return "GIM_Check" + Predicate.getImmTypeIdentifier() + "ImmPredicate";
>>>> + return "GIM_Check" + Predicate.getImmTypeIdentifier().str() + "ImmPredicate";
>>>> }
>>>>
>>>> /// This class stands in for LLT wherever we want to tablegen-erate an
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://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/20171015/43bac0fc/attachment.html>
More information about the llvm-commits
mailing list