This is an archive of the discontinued LLVM Phabricator instance.

Fix load-store optimizer on thumbv4t
ClosedPublic

Authored by jroelofs on Dec 10 2014, 1:40 PM.

Details

Summary

This is another instance of the lo->lo copy problem. Unfortunately, I don't have a testcase.

Diff Detail

Event Timeline

jroelofs updated this revision to Diff 17121.Dec 10 2014, 1:40 PM
jroelofs retitled this revision from to Fix load-store optimizer on thumbv4t.
jroelofs updated this object.
jroelofs edited the test plan for this revision. (Show Details)
jroelofs added reviewers: jmolloy, mroth, t.p.northover.
jroelofs added a subscriber: Unknown Object (MLST).

So why couldn't you create a test? It doesn't seem like a terribly difficult issue to trigger: just a generic load/store merge operation on Thumb1.

llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp
574 ↗(On Diff #17121)

Isn't the whole block already isThumb1?

jroelofs updated this revision to Diff 18554.Jan 21 2015, 2:24 PM
jroelofs updated this object.
jroelofs edited edge metadata.

Add a testcase. Address Tim's comments.

t.p.northover accepted this revision.Jan 21 2015, 2:29 PM
t.p.northover edited edge metadata.

Hi Jon,

Thanks for adding the test. Just one debugging line that crept in. Feel free to commit with that removed.

Tim.

lib/Target/ARM/ARMLoadStoreOptimizer.cpp
564

Probably best without this.

This revision is now accepted and ready to land.Jan 21 2015, 2:29 PM
jroelofs closed this revision.Jan 21 2015, 2:41 PM

Good catch, thanks Tim! r226711

jroelofs added a subscriber: hansw.Jan 22 2015, 3:22 PM

@t.p.northover, @hansw Is r226711 appropriate for the 3.6.0 branch?

hans added a subscriber: hans.Jan 23 2015, 9:13 AM

If Tim is ok with it, I'm happy to merge it.