[PATCH] D62644: [EarlyCSE] Ensure equal keys have the same hash value

[Joseph Tremoulet via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Transforms/Scalar/EarlyCSE.cpp:83-86
+static cl::opt<bool> EarlyCSEDebugHash(
+    "earlycse-debug-hash", cl::init(false), cl::Hidden,
+    cl::desc("Perform extra assertion checking to verify that SimpleValue's hash "
+             "function is well-behaved w.r.t. its isEqual predicate"));
----------------
spatel wrote:
> Since the code has an ifdef guard, do we want the ifdef guard on the option itself? 
> Or just remove the guard at the use, so we can use/test this option even with a release build?
My thinking was:
 1. We don't want an ifdef guard on the option itself, because if we had one then running lit on commute.ll with a release build of opt would fail
 2. We do want a guard on the use, to avoid the run-time cost of repeatedly checking the static every time we compute a SimpleValue hash in release builds (and returning the const from the hash function in release builds isn't that valuable since the point of doing it is to exercise the assertion in isEqual that won't exist in release builds anyway)

I could be convinced to change it (especially if there's already some convention for debug-only flags in lit tests), but that was my reasoning.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62644