Page MenuHomePhabricator

[BDCE] Remove instructions without demanded bits
ClosedPublic

Authored by nikic on Jan 1 2019, 3:40 AM.

Details

Summary

Followup to D55563.

If an instruction has no demanded bits, remove it directly during BDCE, instead of leaving it for something else to clean up.

Diff Detail

Repository
rL LLVM

Event Timeline

nikic created this revision.Jan 1 2019, 3:40 AM
lebedev.ri added inline comments.
test/Transforms/BDCE/invalidate-assumptions.ll
88 ↗(On Diff #179793)

Can you please do a NFC regen commit first?

nikic added inline comments.Jan 1 2019, 3:57 AM
test/Transforms/BDCE/invalidate-assumptions.ll
88 ↗(On Diff #179793)

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?

lebedev.ri added inline comments.Jan 1 2019, 4:08 AM
test/Transforms/BDCE/invalidate-assumptions.ll
88 ↗(On Diff #179793)

[[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.

nikic updated this revision to Diff 179794.Jan 1 2019, 4:41 AM
nikic retitled this revision from [BDCE] Remove intructions without demanded bits to [BDCE] Remove instructions without demanded bits.

Rebase over regenerated test checks.

spatel added inline comments.Jan 1 2019, 7:39 AM
test/Transforms/BDCE/invalidate-assumptions.ll
88 ↗(On Diff #179793)

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.

hfinkel accepted this revision.Jan 1 2019, 10:03 AM

LGTM

This revision is now accepted and ready to land.Jan 1 2019, 10:03 AM
This revision was automatically updated to reflect the committed changes.