Page MenuHomePhabricator

[ARM] Make InstrEmitter mark CPSR defs dead for Thumb1.

Authored by efriedma on Oct 19 2018, 2:22 PM.



The "dead" markings allow existing target-independent optimizations, like MachineSink, to trigger more frequently. The CPSR defs would have eventually been marked dead by LiveVariables, so this only affects optimizations before regalloc.

The ARMBaseInstrInfo.cpp is fixing a bug which is only visible with this change: the transform adds a use to an otherwise dead def of CPSR. This is covered by existing regression tests.

Saves roughly 0.1% codesize on a Thumb1 codebase I've been looking at.

Loosely depends on D53452; one regression test related to TBB formation breaks without that change.

Diff Detail


Event Timeline

efriedma created this revision.Oct 19 2018, 2:22 PM

Interesting. I didn't realise it worked like that. I had presumed that lot of passes would have to be taught about the optional defs, as opposed to them not being marked as dead correctly.

In my testing, the codesize is kind of up and down, looking a little bit up overall. It's roughly a wash, with one big increase in bzip for some reason.

But the performance looks like a big improvement. It fixes the regression we got from rL339472 and shows lots of other I'd previously attributed to D33935. To the extent that D33935 on top of this actually looks it hurts more than it helps. I'm not sure why yet, I would expect them to help each other out more than they hurt each other.

dmgreen accepted this revision.Oct 22 2018, 3:18 AM

Anyway, this looks good to me.

If you want to take a look a bzip's decompress from csibe and figure out why that's increasing, it may help the codesize more, but that shouldn't block this with the other improvements it shows.

Perhaps leave for a day or two in case other people have comments.

This revision is now accepted and ready to land.Oct 22 2018, 3:18 AM
This revision was automatically updated to reflect the committed changes.