Page MenuHomePhabricator

[ARM] Preserve liveness in ARMConstantIslands.
ClosedPublic

Authored by efriedma on Aug 15 2019, 4:22 PM.

Details

Summary

We currently don't use liveness information after this point, but it can be useful to catch bugs using -verify-machineinstrs, and optimizations could potentially use this information in the future.

This is the liveness patch I mentioned in D66243, that caught a miscompile.

Diff Detail

Event Timeline

efriedma created this revision.Aug 15 2019, 4:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 15 2019, 4:22 PM
SjoerdMeijer added inline comments.Aug 15 2019, 11:23 PM
lib/Target/ARM/ARMConstantIslandPass.cpp
881 ↗(On Diff #215495)

nit: "at I" -> "at MI"?

test/CodeGen/ARM/constant-island-movwt.mir
901 ↗(On Diff #215495)

Would it be useful to have, or add a test, that also has some liveins?

efriedma marked an inline comment as done.Aug 16 2019, 11:36 AM
efriedma added inline comments.
test/CodeGen/ARM/constant-island-movwt.mir
901 ↗(On Diff #215495)

This block has live-ins; I just didn't copy-paste them in because listing them didn't seem useful. I can do that if you think it would be helpful, though.

efriedma updated this revision to Diff 215648.Aug 16 2019, 11:40 AM

Address review comments.

SjoerdMeijer accepted this revision.Aug 16 2019, 11:52 AM

his block has live-ins; I just didn't copy-paste them in because listing them didn't seem useful.

I wasn't sure either, so it wasn't a rhetorical question, but at least we do see some liveins now. :-)

Nice patch, LGTM

This revision is now accepted and ready to land.Aug 16 2019, 11:52 AM
This revision was automatically updated to reflect the committed changes.