[DAGCombiner] Use getTokenFactor in a few more cases.

[fhahn]

Summary

SDNodes can only have 64k operands and for some inputs (e.g. large
number of stores), we can reach this limit when creating TokenFactor
nodes. This patch is a follow up to D56740 and updates a few more places
that potentially can create TokenFactors with too many operands.

[RKSimon]

Any chance of test cases?

[fhahn]

Unfortunately this requires at least 64k stores to trigger AFAIK. It is triggered by the same input as D56740. I could try to reduce it, but I am not sure if such a big test case would be valuable?

[RKSimon]

TBH I didn't expect that tests would be easy, but I don't think that should be a stopper to this patch (and code coverage indicates that all these cases are active).

LGTM

[fhahn]

In D59156#1429460, @RKSimon wrote:

TBH I didn't expect that tests would be easy, but I don't think that should be a stopper to this patch (and code coverage indicates that all these cases are active).

Thanks! I'll try to reduce it over the weekend. Also, this might be something that would be good to fuzz.

[fhahn]

In D59156#1429460, @RKSimon wrote:

TBH I didn't expect that tests would be easy, but I don't think that should be a stopper to this patch (and code coverage indicates that all these cases are active).

I've got a simple python script to generate a test case, but it takes multiple minutes, because DAGCombine is really slow with lots of stores (which is another thing I am looking into). For now, I think the patch has to go in without test case :(

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

if (Changed) {
     } else {
       if (DidPruneOps) {
         SmallVector<SDValue, 8> PrunedOps;
         //
         for (const SDValue &Op : Ops) {
           if (SeenChains.count(Op.getNode()) == 0)
             PrunedOps.push_back(Op);
         }
-        Result = DAG.getNode(ISD::TokenFactor, SDLoc(N), MVT::Other, PrunedOps);
+        Result = DAG.getTokenFactor(SDLoc(N), PrunedOps);
       } else {
-        Result = DAG.getNode(ISD::TokenFactor, SDLoc(N), MVT::Other, Ops);
+        Result = DAG.getTokenFactor(SDLoc(N), Ops);
       }
     }
     return Result;
   }
   return SDValue();
 }
 
 /// MERGE_VALUES can always be eliminated.
SDValue DAGCombiner::getMergeStoreChains(SmallVectorImpl<MemOpLink> &StoreNodes,
 
   // don't include nodes that are children
   for (unsigned i = 0; i < NumStores; ++i) {
     if (Visited.count(StoreNodes[i].MemNode->getChain().getNode()) == 0)
       Chains.push_back(StoreNodes[i].MemNode->getChain());
   }
 
   assert(Chains.size() > 0 && "Chain should have generated a chain");
-  return DAG.getNode(ISD::TokenFactor, StoreDL, MVT::Other, Chains);
+  return DAG.getTokenFactor(StoreDL, Chains);
 }
 
 bool DAGCombiner::MergeStoresOfConstantsOrVecElts(
     SmallVectorImpl<MemOpLink> &StoreNodes, EVT MemVT, unsigned NumStores,
     bool IsConstantSrc, bool UseVector, bool UseTrunc) {
   // Make sure we have something to merge.
   if (NumStores < 2)
     return false;
SDValue DAGCombiner::FindBetterChain(LSBaseSDNode *N, SDValue OldChain) {
   if (Aliases.size() == 0)
     return DAG.getEntryNode();
 
   // If a single operand then chain to it.  We don't need to revisit it.
   if (Aliases.size() == 1)
     return Aliases[0];
 
   // Construct a custom tailored token factor.
-  return DAG.getNode(ISD::TokenFactor, SDLoc(N), MVT::Other, Aliases);
+  return DAG.getTokenFactor(SDLoc(N), Aliases);
 }
 
 namespace {
 // TODO: Replace with with std::monostate when we move to C++17.
 struct UnitT { } Unit;
 bool operator==(const UnitT &, const UnitT &) { return true; }
 bool operator!=(const UnitT &, const UnitT &) { return false; }
 } // namespace