Page MenuHomePhabricator

[ARM] Add some missing thumb1 opcodes to enable peephole optimisation of CMPs

Authored by dmgreen on Feb 6 2019, 11:27 AM.

Diff Detail


Event Timeline

dmgreen created this revision.Feb 6 2019, 11:27 AM
dmgreen planned changes to this revision.Feb 6 2019, 11:37 AM
dmgreen marked an inline comment as done.
dmgreen added inline comments.
2923 ↗(On Diff #185597)

Whoops. Forgot about this bit.

efriedma added inline comments.Feb 6 2019, 1:31 PM
2624 ↗(On Diff #185597)

Can you commit the reformatting separately? It's a little confusing to distinguish which changes are actually relevant.

2660 ↗(On Diff #185597)

What are these "isReg" checks supposed to do? You already checked the opcode.

dmgreen updated this revision to Diff 185731.Feb 7 2019, 3:21 AM

Fixed that bit I missed, plus better formatting and cleanup

I'd like to see testcases for all the possible add/sub opcodes, if it isn't too much work.

2659 ↗(On Diff #185731)

The corresponding ARM/Thumb2 code checks for ADDrr; should we add the corresponding check for tADDrr?
Or does that not work for some reason?

2679 ↗(On Diff #185731)

I guess it's sort of orthogonal to this patch, but this list isn't complete, if it's supposed to be a list of instructions which update the NZ bits. (Missing tADC, tSBC, tAND, tORR, tEOR, tBIC, tMVN, tASRri, tASRrr, tROR, I think.)

dmgreen added inline comments.Feb 12 2019, 4:25 AM
2659 ↗(On Diff #185731)

Thanks, that was lost in a reshuffle I think. It does show up some machine verifier problems with this. IsThumb1 isn't being set correctly

dmgreen updated this revision to Diff 186643.Feb 13 2019, 6:47 AM

Updates for IsThumb1

This revision is now accepted and ready to land.Feb 13 2019, 12:41 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 21 2019, 2:30 AM