[PATCH] D60982: [SimplifyCFG] Use lookup tables when they are more space efficient or a huge speed win.

[James Molloy via Phabricator via llvm-commits]

jmolloy requested changes to this revision.
jmolloy added inline comments.
This revision now requires changes to proceed.


================
Comment at: include/llvm/Analysis/TargetTransformInfo.h:582
+  /// this target.
+  bool shouldBuildLookupTable(size_t TableSize, uint32_t NumCases,
+                              unsigned CaseSize, bool HasOptSize) const;
----------------
To be honest, I'd really just express this as:

  // Return true if the given SwichInst should be converted into a lookup table. The size of the lookup table is \c TableSize and the number of covered cases is \c NumCases (meaning the number of table entries that hit the default case is \c TableSize - \c NumCases).
  bool shouldBuildLookupTable(SwitchInst *SI, uint64_t TableSize, unsigned NumCases);

Sorry for the churn here, I know I didn't mention this in your previous review. But passing the SwitchInst itself and letting the target fish out OptSize/SI->getType()->getIntegerBitWidth() seems cleaner.

Also, never use size_t or uint32_t here. TableSize's maximum extent isn't host dependent, it's target dependent. Use uint64_t to guarantee 64-bit coverage on all hosts. Similarly NumCases isn't limited to 32-bits, so use unsigned because that's what we use everywhere.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60982/new/

https://reviews.llvm.org/D60982