Page MenuHomePhabricator

Fix PR18364: Only emit movw for ARM architectures that support it
ClosedPublic

Authored by dim on Sep 2 2014, 11:21 AM.

Details

Summary

See http://llvm.org/PR18364 for the initial report and discussion.

The fix is to add a HasV6T2 check to the ARMadde GPR:$src, imm0_65535_neg pattern, and add a testcase.

The only additional problem is that test/CodeGen/ARM/carry.ll fails after this change, because that test does not seem to emit a movw anymore. This may well have to do with the default ARM CPU chosen on a certain host system. The solution is probably to add a more explicit architecture in the llc command for this test. Suggestions welcome.

Diff Detail

Event Timeline

dim updated this revision to Diff 13174.Sep 2 2014, 11:21 AM
dim retitled this revision from to Fix PR18364: Only emit movw for ARM architectures that support it.
dim updated this object.
dim edited the test plan for this revision. (Show Details)
dim added reviewers: rengolin, t.p.northover.
dim added a subscriber: Unknown Object (MLST).Sep 2 2014, 11:24 AM
rengolin edited edge metadata.Sep 2 2014, 11:27 AM

I agree you should add a more specific target for the failing test.

cheers,
--renato

test/CodeGen/ARM/pr18364-movw.ll
3

a bit pedantic, but can you add armv6t2? just to clear the boundaries

dim updated this revision to Diff 13175.Sep 2 2014, 11:46 AM
dim edited edge metadata.

Per Renato's suggestions:

  • Use armv6t2-eabi for test/CodeGen/ARM/carry.ll, so it may emit movw
  • Add explicit armv6t2 test to test/CodeGen/ARM/pr18364-movw.ll
rengolin accepted this revision.Sep 2 2014, 12:30 PM
rengolin edited edge metadata.

LGTM, thanks!

--renato

This revision is now accepted and ready to land.Sep 2 2014, 12:30 PM
dim added a comment.Sep 2 2014, 1:40 PM

It would be nice if somebody could commit it for me, as I don't have commit privileges. :)

rengolin closed this revision.Sep 2 2014, 3:56 PM

Committed in r216989 and r216990 (I missed the test).

cheers,
--renato