[PATCH] D64176: [Bugpoint redesign] Added Pass to Remove Global Variables

[Diego Treviño via Phabricator via llvm-commits]

diegotf added inline comments.


================
Comment at: llvm/test/Reduce/remove-global-vars.ll:18-32
+  store i32 0, i32* %retval, align 4
+  ; CHECK-NOT: load i32, i32* @uninteresting2, align 4
+  %0 = load i32, i32* @uninteresting2, align 4
+  store i32 %0, i32* @interesting, align 4
+  ; CHECK-NOT: load i32, i32* @uninteresting3, align 4
+  %1 = load i32, i32* @uninteresting3, align 4
+  %dec = add nsw i32 %1, -1
----------------
dblaikie wrote:
> Should these CHECK-NOTs also have some CHECKs that demonstrate what these have been transformed into? (showing the undefs appear)
> 
> Also - are there differences between uninteresting1/2/3 that's interesting/important to testing here, or could there be similar test coverage with 1 uninteresting/removed global?
Agreed, i'll add the checks. And yeah, uninteresting 2&3 might be overkill. I'll update the test accordingly


================
Comment at: llvm/tools/llvm-reduce/deltas/Delta.cpp:123-126
+  std::vector<Chunk> Chunks = {{1, Targets}};
+  std::set<Chunk> UninterestingChunks;
+  std::unique_ptr<Module> ReducedProgram;
+
----------------
dblaikie wrote:
> This change appears independent of the rest of this patch - if that's the case, please commit it separately. (no need to send it for review if it's nothing too interesting - if it's just reducing the scope of variables as is generally good)
I'll move it to another then :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64176