[PATCH] D43601: [bugpoint] Add NoStripSymbols option.

Matthias Braun via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 22 16:14:03 PST 2018


MatzeB added inline comments.


================
Comment at: include/llvm/Transforms/IPO.h:34
 
+bool StripSymbolNames(Module &M, bool PreserveDbgInfo);
+
----------------
This needs a doxygen comment.


================
Comment at: lib/Transforms/IPO/StripSymbols.cpp:206
 /// StripSymbolNames - Strip symbol names.
-static bool StripSymbolNames(Module &M, bool PreserveDbgInfo) {
+bool StripSymbolNames(Module &M, bool PreserveDbgInfo) {
 
----------------
You could go for `llvm::StripSymbolNames()` instead of the `namespace llvm {}` if it's just this one function (just FYI I don't care either way).


================
Comment at: test/BugPoint/strip-names-and-types.ll:16-17
+  %A = icmp eq %type0* undef, undef
+; CHECK: icmp eq %0* undef, undef
+; DISABLE: icmp eq %type0* undef, undef
+  call i1 @foo(i1 %A)
----------------
This is also stripping symbols from SSA values and not just from types isn't it? The test should check at least a def and use of an SSA value then I think.


================
Comment at: tools/bugpoint/CrashDebugger.cpp:73
 
+static bool stripSymbolNames(Module &M) { return StripSymbolNames(M, true); }
+
----------------
This seems superfluous given that it only has a single caller.


Repository:
  rL LLVM

https://reviews.llvm.org/D43601





More information about the llvm-commits mailing list