Remove unnecessary idiom recognition assertions

There are uses of TR_ASSERT() in idiom recognition that check for cases
that are not necessarily impossible and that are handled conservatively
in builds where TR_ASSERT() is not checked. Assertions are not a good
way to detect and report these cases, since assertion failure is not
likely to be due to a bug.

This commit removes a number of these assertions. Now the conservative
logic will apply in all builds. When it does, a static debug counter
will be incremented, and (when tracing idiom recognition) a message will
be printed to the log.

Anyone who wants to look for transformations that have been prevented
due to incomplete support for a particular case can use the new static
debug counters.

Additionally, use dumpOptDetails() to trace when a transformer fails.
In that case it's misleading to leave the log showing only the message
from performTransformation().

runtime/compiler/optimizer/IdiomRecognition.cpp

@@ -1945,7 +1945,8 @@ TR_CISCTransformer::TR_CISCTransformer(TR::OptimizationManager *manager)
      _bblistSucc(manager->trMemory()),
      _candidatesForRegister(manager->trMemory()),
      _useTreeTopMap(manager->comp(), manager->optimizer()),
-     _BitsKeepAliveList(manager->trMemory())
+     _BitsKeepAliveList(manager->trMemory()),
+     _countFailBuf(manager->trMemory()->currentStackRegion())
    {
    _afterInsertionsIdiom = (ListHeadAndTail<TR::Node> *) trMemory()->allocateHeapMemory(sizeof(ListHeadAndTail<TR::Node>)*2);
    memset(_afterInsertionsIdiom, 0, sizeof(ListHeadAndTail<TR::Node>)*2);
@@ -6689,8 +6690,8 @@ TR_CISCTransformer::analyzeBoolTable(TR_BitVector **bv, TR::TreeTop **retSameExi
                      ntakenBV -= tmpBV;
                      break;
                   default:
-                     TR_ASSERT(false, "not implemented yet");
                      // not implemented yet
+                     countUnhandledOpcode(__FUNCTION__, n->getOpcode());
                      return false;
                   }
 
@@ -6814,7 +6815,8 @@ TR_CISCTransformer::analyzeByteBoolTable(TR_CISCNode *boolTable, uint8_t *table2
    bv = (TR_BitVector **)trMemory()->allocateMemory(size, stackAlloc);
    memset(bv, 0, size);
 
-   switch((defTargetNode ? defTargetNode : defNode)->getOpcode())
+   uint32_t opcode = (defTargetNode ? defTargetNode : defNode)->getOpcode();
+   switch (opcode)
       {
       case TR::b2i:
          if (defNode->isOptionalNode()) defNode = defNode->getChild(0);
@@ -6826,8 +6828,8 @@ TR_CISCTransformer::analyzeByteBoolTable(TR_CISCNode *boolTable, uint8_t *table2
          defBV.setAll(   0+BYTEBVOFFSET, 255+BYTEBVOFFSET);
          break;
       default:
-         TR_ASSERT(false, "not implemented yet");
          // not implemented yet
+         countUnhandledOpcode(__FUNCTION__, opcode);
          return -1;     // error
       }
 
@@ -6903,7 +6905,8 @@ TR_CISCTransformer::analyzeCharBoolTable(TR_CISCNode *boolTable, uint8_t *table6
    bv = (TR_BitVector **)trMemory()->allocateMemory(size, stackAlloc);
    memset(bv, 0, size);
 
-   switch((defTargetNode ? defTargetNode : defNode)->getOpcode())
+   uint32_t opcode = (defTargetNode ? defTargetNode : defNode)->getOpcode();
+   switch (opcode)
       {
       case TR::su2i:
          if (defNode->isOptionalNode()) defNode = defNode->getChild(0);
@@ -6912,8 +6915,8 @@ TR_CISCTransformer::analyzeCharBoolTable(TR_CISCNode *boolTable, uint8_t *table6
          defBV.setAll(0, 65535);
          break;
       default:
-         TR_ASSERT(false, "not implemented yet");
          // not implemented yet
+         countUnhandledOpcode(__FUNCTION__, opcode);
          return -1;     // error
       }
 
@@ -7650,7 +7653,7 @@ TR_CISCTransformer::computeTopologicalEmbedding(TR_CISCGraph *P, TR_CISCGraph *T
    if (performTransformation(comp(), "%sReducing loop %d to %s\n", OPT_DETAILS, _bblistBody.getListHead()->getData()->getNumber(),
                                          P->getTitle()) && !transformer(this))
       {
-      if (trace()) traceMsg(comp(), "computeTopologicalEmbedding: IL Transformer failed. (step 4)\n\n");
+      dumpOptDetails(comp(), "computeTopologicalEmbedding: IL Transformer failed. (step 4)\n\n");
       registerCandidates();
       _T->restoreListsDuplicator();
       return false;             // The transformation fails
@@ -7915,3 +7918,45 @@ TR_CISCTransformer::restoreBitsKeepAliveCalls()
       prev->insertAfter(tt);
       }
    }
+
+void
+TR_CISCTransformer::countFail(const char *fmt, ...)
+   {
+   _countFailBuf.clear();
+
+   va_list args;
+   va_start(args, fmt);
+   _countFailBuf.vappendf(fmt, args);
+   va_end(args);
+
+   // This isn't the best trace message, but it signals that something happened
+   // in the log even when the debug counters aren't enabled, and it avoids
+   // making call sites too cumbersome.
+   if (trace())
+      traceMsg(comp(), "failed: %s\n", _countFailBuf.text());
+
+   TR::DebugCounter::incStaticDebugCounter(
+      comp(),
+      TR::DebugCounter::debugCounterName(
+         comp(),
+         "idiomRecognition.failed/%s/%s/(%s)/%s/loop=%d",
+         _countFailBuf.text(),
+         _P->getTitle(),
+         comp()->signature(),
+         comp()->getHotnessName(comp()->getMethodHotness()),
+         _bblistBody.getListHead()->getData()->getNumber()));
+   }
+
+void
+TR_CISCTransformer::countUnhandledOpcode(const char *where, uint32_t opcode)
+   {
+   if (opcode < TR::NumAllIlOps)
+      {
+      const char *name = TR::ILOpCode((TR::ILOpCodes)opcode).getName();
+      countFail("%s/unhandledOpcode/%s", where, name);
+      }
+   else
+      {
+      countFail("%s/unhandledOpcode/%u", where, opcode);
+      }
+   }

runtime/compiler/optimizer/IdiomRecognition.hpp

@@ -38,6 +38,7 @@
 #include "infra/HashTab.hpp"
 #include "infra/Link.hpp"
 #include "infra/List.hpp"
+#include "infra/String.hpp"
 #include "infra/TRlist.hpp"
 #include "optimizer/LoopCanonicalizer.hpp"
 #include "optimizer/OptimizationManager.hpp"
@@ -1469,6 +1470,9 @@ class TR_CISCTransformer : public TR_LoopTransformer
    TR_RegionStructure *getCurrentLoop() { return _loopStructure; }
    void setCurrentLoop(TR_RegionStructure *loop) { _loopStructure = loop; }
 
+   void countFail(const char *fmt, ...) TR_PRINTF_FORMAT_ATTR(2, 3);
+   void countUnhandledOpcode(const char *where, uint32_t opcode);
+
 private:
    List<TR::Block> _bblistPred;
    List<TR::Block> _bblistBody;
@@ -1510,6 +1514,8 @@ class TR_CISCTransformer : public TR_LoopTransformer
    uint8_t *_DE;        // just for working
    bool _isGenerateI2L;
    bool _showMesssagesStdout;
+
+   TR::StringBuf _countFailBuf;
    };
 
 #endif

runtime/compiler/optimizer/IdiomTransformations.cpp

@@ -2245,8 +2245,11 @@ CISCTransform2NestedArrayFindBytes(TR_CISCTransformer *trans)
 
    TR_ASSERT(trans->getP()->getVersionLength() == 0, "Versioning code is not implemented yet");
 
-   TR_ASSERT(trans->isEmptyAfterInsertionIdiomList(0) && trans->isEmptyAfterInsertionIdiomList(1), "Not implemented yet!");
-   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1)) return false;
+   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1))
+      {
+      trans->countFail("%s/nonemptyAfterInsertionIdiomList", __FUNCTION__);
+      return false;
+      }
 
    trans->findFirstNode(&trTreeTop, &trNode, &block);
    if (!block) return false;    // cannot find
@@ -3625,8 +3628,11 @@ CISCTransform2TROTArray(TR_CISCTransformer *trans)
    List<TR_CISCNode> *P2T = trans->getP2T();
    TR::Compilation *comp = trans->comp();
 
-   TR_ASSERT(trans->isEmptyAfterInsertionIdiomList(0) && trans->isEmptyAfterInsertionIdiomList(1), "Not implemented yet!");
-   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1)) return false;
+   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1))
+      {
+      trans->countFail("%s/nonemptyAfterInsertionIdiomList", __FUNCTION__);
+      return false;
+      }
 
    trans->findFirstNode(&trTreeTop, &trNode, &block);
    if (!block) return false;    // cannot find
@@ -5035,8 +5041,11 @@ CISCTransform2TRTOArray(TR_CISCTransformer *trans)
    List<TR_CISCNode> *P2T = trans->getP2T();
    TR::Compilation *comp = trans->comp();
 
-   TR_ASSERT(trans->isEmptyAfterInsertionIdiomList(0) && trans->isEmptyAfterInsertionIdiomList(1), "Not implemented yet!");
-   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1)) return false;
+   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1))
+      {
+      trans->countFail("%s/nonemptyAfterInsertionIdiomList", __FUNCTION__);
+      return false;
+      }
 
    trans->findFirstNode(&trTreeTop, &trNode, &block);
    if (!block) return false;    // cannot find
@@ -5832,8 +5841,11 @@ CISCTransform2ArrayCopySub(TR_CISCTransformer *trans, TR::Node *indexRepNode, TR
    bool isDecrement = trans->isMEMCPYDec();
    const bool disptrace = DISPTRACE(trans);
 
-   TR_ASSERT(trans->isEmptyAfterInsertionIdiomList(0) && trans->isEmptyAfterInsertionIdiomList(1), "Not implemented yet!");
-   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1)) return false;
+   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1))
+      {
+      trans->countFail("%s/nonemptyAfterInsertionIdiomList", __FUNCTION__);
+      return false;
+      }
 
    trans->findFirstNode(&trTreeTop, &trNode, &block);
    if (!block) return false;    // cannot find
@@ -6347,8 +6359,11 @@ CISCTransform2ArrayCopyB2CorC2B(TR_CISCTransformer *trans)
 
    TR_ASSERT(trans->getP()->getVersionLength() == 0, "Versioning code is not implemented yet");
 
-   TR_ASSERT(trans->isEmptyAfterInsertionIdiomList(0) && trans->isEmptyAfterInsertionIdiomList(1), "Not implemented yet!");
-   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1)) return false;
+   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1))
+      {
+      trans->countFail("%s/nonemptyAfterInsertionIdiomList", __FUNCTION__);
+      return false;
+      }
 
    trans->findFirstNode(&trTreeTop, &trNode, &block);
    if (!block) return false;    // cannot find
@@ -6703,8 +6718,11 @@ CISCTransform2ArrayCopyB2CBndchk(TR_CISCTransformer *trans)
    List<TR_CISCNode> *P2T = trans->getP2T();
    TR::Compilation *comp = trans->comp();
 
-   TR_ASSERT(trans->isEmptyAfterInsertionIdiomList(0) && trans->isEmptyAfterInsertionIdiomList(1), "Not implemented yet!");
-   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1)) return false;
+   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1))
+      {
+      trans->countFail("%s/nonemptyAfterInsertionIdiomList", __FUNCTION__);
+      return false;
+      }
 
    trans->findFirstNode(&trTreeTop, &trNode, &block);
    if (!block) return false;    // cannot find
@@ -6919,8 +6937,11 @@ CISCTransform2ArrayCopyC2BMixed(TR_CISCTransformer *trans)
    List<TR_CISCNode> *P2T = trans->getP2T();
    TR::Compilation *comp = trans->comp();
 
-   TR_ASSERT(trans->isEmptyAfterInsertionIdiomList(0) && trans->isEmptyAfterInsertionIdiomList(1), "Not implemented yet!");
-   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1)) return false;
+   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1))
+      {
+      trans->countFail("%s/nonemptyAfterInsertionIdiomList", __FUNCTION__);
+      return false;
+      }
 
    trans->findFirstNode(&trTreeTop, &trNode, &block);
    if (!block) return false;    // cannot find
@@ -7249,8 +7270,11 @@ CISCTransform2ArrayCopyC2BIf2(TR_CISCTransformer *trans)
 
    TR_ASSERT(trans->getP()->getVersionLength() == 0, "Versioning code is not implemented yet");
 
-   TR_ASSERT(trans->isEmptyAfterInsertionIdiomList(0) && trans->isEmptyAfterInsertionIdiomList(1), "Not implemented yet!");
-   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1)) return false;
+   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1))
+      {
+      trans->countFail("%s/nonemptyAfterInsertionIdiomList", __FUNCTION__);
+      return false;
+      }
 
    trans->findFirstNode(&trTreeTop, &trNode, &block);
    if (!block) return false;    // cannot find
@@ -7533,8 +7557,11 @@ CISCTransform2ArrayCopyB2I(TR_CISCTransformer *trans)
 
    TR_ASSERT(trans->getP()->getVersionLength() == 0, "Versioning code is not implemented yet");
 
-   TR_ASSERT(trans->isEmptyAfterInsertionIdiomList(0) && trans->isEmptyAfterInsertionIdiomList(1), "Not implemented yet!");
-   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1)) return false;
+   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1))
+      {
+      trans->countFail("%s/nonemptyAfterInsertionIdiomList", __FUNCTION__);
+      return false;
+      }
 
    trans->findFirstNode(&trTreeTop, &trNode, &block);
    if (!block) return false;    // cannot find
@@ -8010,8 +8037,11 @@ CISCTransform2ArraySet(TR_CISCTransformer *trans)
    TR::Compilation *comp = trans->comp();
    bool ctrl = trans->isGenerateI2L();
 
-   TR_ASSERT(trans->isEmptyAfterInsertionIdiomList(0) && trans->isEmptyAfterInsertionIdiomList(1), "Not implemented yet!");
-   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1)) return false;
+   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1))
+      {
+      trans->countFail("%s/nonemptyAfterInsertionIdiomList", __FUNCTION__);
+      return false;
+      }
 
    trans->findFirstNode(&trTreeTop, &trNode, &block);
    if (!block) return false;    // cannot find
@@ -8678,8 +8708,11 @@ CISCTransform2MixedArraySet(TR_CISCTransformer *trans)
 
    TR_ASSERT(trans->getP()->getVersionLength() == 0, "Versioning code is not implemented yet");
 
-   TR_ASSERT(trans->isEmptyAfterInsertionIdiomList(0) && trans->isEmptyAfterInsertionIdiomList(1), "Not implemented yet!");
-   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1)) return false;
+   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1))
+      {
+      trans->countFail("%s/nonemptyAfterInsertionIdiomList", __FUNCTION__);
+      return false;
+      }
 
    trans->findFirstNode(&trTreeTop, &trNode, &block);
    if (!block) return false;    // cannot find
@@ -10270,8 +10303,11 @@ CISCTransform2BitOpMem(TR_CISCTransformer *trans)
    TR::Compilation *comp = trans->comp();
    bool ctrl = trans->isGenerateI2L();
 
-   TR_ASSERT(trans->isEmptyAfterInsertionIdiomList(0) && trans->isEmptyAfterInsertionIdiomList(1), "Not implemented yet!");
-   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1)) return false;
+   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1))
+      {
+      trans->countFail("%s/nonemptyAfterInsertionIdiomList", __FUNCTION__);
+      return false;
+      }
 
    trans->findFirstNode(&trTreeTop, &trNode, &block);
    if (!block) return false;    // cannot find
@@ -10805,8 +10841,11 @@ CISCTransform2CountDecimalDigit(TR_CISCTransformer *trans)
 
    TR_ASSERT(trans->getP()->getVersionLength() == 0, "Versioning code is not implemented yet");
 
-   TR_ASSERT(trans->isEmptyAfterInsertionIdiomList(0) && trans->isEmptyAfterInsertionIdiomList(1), "Not implemented yet!");
-   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1)) return false;
+   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1))
+      {
+      trans->countFail("%s/nonemptyAfterInsertionIdiomList", __FUNCTION__);
+      return false;
+      }
 
    trans->findFirstNode(&trTreeTop, &trNode, &block);
    if (!block) return false;    // cannot find
@@ -11114,8 +11153,11 @@ CISCTransform2LongToStringDigit(TR_CISCTransformer *trans)
 
    TR_ASSERT(trans->getP()->getVersionLength() == 0, "Versioning code is not implemented yet");
 
-   TR_ASSERT(trans->isEmptyAfterInsertionIdiomList(0) && trans->isEmptyAfterInsertionIdiomList(1), "Not implemented yet!");
-   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1)) return false;
+   if (!trans->isEmptyAfterInsertionIdiomList(0) || !trans->isEmptyAfterInsertionIdiomList(1))
+      {
+      trans->countFail("%s/nonemptyAfterInsertionIdiomList", __FUNCTION__);
+      return false;
+      }
 
    trans->findFirstNode(&trTreeTop, &trNode, &block);
    if (!block) return false;    // cannot find