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

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added inline comments.


================
Comment at: llvm/include/llvm/Analysis/StructuralHash.h:1
+//=- StructuralHash.h - Structural Hash Printing --*- C++ -*---------------0-=//
+//
----------------



================
Comment at: llvm/include/llvm/Analysis/StructuralHash.h:9
+//
+// This file defines the StructuralHashPrinterPass
+//
----------------
this comment doesn't seem very useful, I'd remove it


================
Comment at: llvm/lib/Analysis/StructuralHash.cpp:20
+
+cl::opt<bool> EnableDetailedStructuralHashing(
+    "enable-detailed-structural-hash", cl::Hidden, cl::init(false),
----------------
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`


================
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})}}
----------------
this and `DETAILED-HASH` are the same CHECK lines. did you mean to check that `DETAILED-HASH` gives two different hashes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158317