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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/test/Reduce/remove-global-vars.ll:8-14
+; CHECK-NOT: @uninteresting1
+ at uninteresting1 = global i32 0, align 4
+; CHECK: @interesting
+ at interesting = global i32 5, align 4
+; CHECK-NOT: @uninteresting2
+ at uninteresting2 = global i32 25, align 4
+; CHECK-NOT: @uninteresting3
----------------
diegotf wrote:
> dblaikie wrote:
> > Might be stronger to:
> > 
> > CHECK-NOT: "global" on either side of the "CHECK: @interesting = global"
> > 
> > That way it'd be resilient to any reordering, for instance (where you could silently pass the test if LLVM IR happened to print 2, 3, unnumbered, 1)
> Good catch! I hadn't thought of that.
seems good to have it on both sides (CHECK-NOT before and after the CHECK @interesting).


================
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
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64176