[PATCH] D56185: [BDCE] Remove instructions without demanded bits

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: test/Transforms/BDCE/invalidate-assumptions.ll:88
 ; CHECK-LABEL: @PR34179(
-; CHECK-NEXT:    [[T0:%.*]] = load volatile i32, i32* %a
+; CHECK-NEXT:    [[T0:%.*]] = load volatile i32, i32* [[A:%.*]]
 ; CHECK-NEXT:    ret void
----------------
lebedev.ri wrote:
> nikic wrote:
> > lebedev.ri wrote:
> > > Can you please do a NFC regen commit first?
> > I'm wondering if I should maybe just manually edit those changes away? Using a pattern for an argument name doesn't make a lot of sense to me. Is there a reason why update_test_checks.py generates it this way? Is this so the result looks for uniform?
> `[[A:%.*]]` means "match `%.*` regex, and store it to `A`".
> So when `[[A]]` is encountered next, it will be expanded to what was previously matched, which is `%a`.
> And no, consistently using these update tools is the only sane way.
I agree that using a pattern for an argument is not ideal. The history for this is that the script used to *not* regex-ify arguments (that was my quick/dirty initial implementation which was just copied from existing scripts), but the script also didn't work with tests that had loops or function calls. This was fixed with D28384 and follow-up commits. 

I don't know what it takes to improve the script to do both things, but it would be a welcome change if someone wants to take that on. Independent of that, I agree with Roman - just use the script as-is to update regression tests. It's less error-prone to run the script than hand-edit small improvements on top of the results.


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

https://reviews.llvm.org/D56185