[PATCH] D31080: [DAG] Extract switch lowering as a spearate object NFC

[Jun Bum Lim via Phabricator via llvm-commits]

junbuml added a comment.

Thanks Hans for the review.

> Would it be possible instead to do the lowering using just the CaseCluster objects, and maybe storing any auxiliary data on the side?

My initial thought was to have only Kind and CaseVector in CaseCluster as we can extract Low and High from the CaseVector. However, this may increase runtime cost as we often access the Low and High, so I cached them in the struct itself.

MachineCaseCluster has two more fields 1) BranchProbability and 2) the union for the destination block or the case index in JT/BT. We can introduce an auxiliary structure to map clusters with the information, but I think this may increase code complexity and cost to access them through  the  auxiliary map. Instead of an auxiliary data for BranchProbability and the union, what about to hold a pointer to CaseCluster in MachineCaseCluster like :

  struct MachineCaseCluster {
    CaseCluster *Cluster;
    union {
      MachineBasicBlock *MBB;
      unsigned JTCasesIndex;
      unsigned BTCasesIndex;
    };
    BranchProbability Prob;
  }

> Before we get any further, I also would like to ask if you have done any measurements of compile-time with this set of patches. As I said before, I think this be quite an expensive hook to call for the inline cost analysis, and it would be nice to see some numbers. If it turns out that it is expensive, perhaps we could come up with some better inline cost heuristic, perhaps something based on the density of the switch.

Sure, compile-time experiment should be reported with the patches. Regarding the cost for the hook, I want to discuss in https://reviews.llvm.org/D31085 in which the hook is introduced. I will copy your comment in https://reviews.llvm.org/D31085.


https://reviews.llvm.org/D31080