Page MenuHomePhabricator

[InstCombine] Simplify shift-by-sext to shift-by-zext - ignore use count on sext
AbandonedPublic

Authored by lebedev.ri on Sep 27 2019, 11:05 AM.

Details

Reviewers
efriedma
spatel
Summary

In D68103 the InstCombine learned that shift-by-sext is simply a shift-by-zext.
But the transform is limited to single-use sext.
That is too limiting. We should handle cases with extra uses.
Here it is proposed to just always do the transformation,
on the basis that zext is unarguably better for any further analysis.

There are other alternatives

  • Simply ensure that all uses of this sext are shifts - O(N^2) in case if there are non-shift uses
  • Add more greedy version of the fold into AggressiveInstCombine - checking that all uses can be converted to zext - could be as simple as again simply checking that they are all shifts. Will not have O(N^2) problem, but will have usability problem - AggressiveInstCombine only runs in -O3, not even in -O2?

So i suspect this might be the least of the evils..

Diff Detail

Event Timeline

lebedev.ri created this revision.Sep 27 2019, 11:05 AM
lebedev.ri edited the summary of this revision. (Show Details)

Also perform sudo-CSE to avoid creating numerous zexts and leaving them up to the next time EarlyCSE.
This obviously doesn't help with pre-existing zexts..

@efriedma any thoughts on this?

lebedev.ri abandoned this revision.Jan 25 2020, 3:27 AM