[PATCH] D158317: [IR] Add SturcturalHash printer pass

[Aiden Grossman via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Analysis/StructuralHash.cpp:20
+
+cl::opt<bool> EnableDetailedStructuralHashing(
+    "enable-detailed-structural-hash", cl::Hidden, cl::init(false),
----------------
aeubanks wrote:
> this should be a textual pass parameter, see `FUNCTION_PASS_WITH_PARAMS`. typically `parseSinglePassOption` expects `foo<bar>`, and this pass name is `print<foo<bar>>`, so you'll need to strip off the `print<>` before passing the name to `parseSinglePassOption`
Updated. I ended up doing it similar to how it's done for `print<memoryssa>` where the syntax is `print<pass-name><option>` which seems to already have support within `parseSinglePassOptions` and is at least somewhat canonical.


================
Comment at: llvm/test/Analysis/StructuralHash/structural-hash-printer.ll:14
+
+; CHECK: Module Hash: {{([a-z0-9]{16})}}
+; CHECK: Function f1 Hash: {{([a-z0-9]{16})}}
----------------
aeubanks wrote:
> this and `DETAILED-HASH` are the same CHECK lines. did you mean to check that `DETAILED-HASH` gives two different hashes?
The original intention here was just to make sure that they were printing anything at all rather than be flaky in response to a specific hash, but that's a good point. I've adjusted it to specific hashes as that should be rather consistent (other than in response to changes in `StructuralHash`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158317