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

[Shawn Landden via Phabricator via llvm-commits]

shawnl marked an inline comment as done.
shawnl added inline comments.


================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:5183
+
+  // The table density should be at least 33% for 64-bit integers.
   // FIXME: Find the best cut-off.
----------------
jmolloy wrote:
> But why? You've replaced 40% with 33% and you mention 64-bit integers but the following heuristic doesn't use 64-bit integers anywhere.
Along with the comment on the multiplication overflow above: this is basic grade-school multiplication. 1/3 == 33 %; 64 / 8 == 8.

The heuristic has threatened to de-rail this patch set with bike-shedding, and has made me really frustrated.

Communication is a key part of doing this work, but when you worry that multiplication will not be understood by the reviewer it really throws a wrench in the process.


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

https://reviews.llvm.org/D60982