Page MenuHomePhabricator

[InstCombine] fold add(zext(xor X, C), C) --> sext X when C is INT_MIN in the source type
ClosedPublic

Authored by spatel on Jul 18 2016, 2:18 PM.

Details

Summary

The pattern may look more obviously like a sext if written as:

define i32 @g(i16 %x) {
  %zext = zext i16 %x to i32
  %xor = xor i32 %zext, 32768
  %add = add i32 %xor, -32768
  ret i32 %add
}

We already have that fold in visitAdd().

And that probably gives away how I got here: I was testing if we had any missing folds if we make D22271 more liberal about pulling logic ops ahead of zexts.

Diff Detail

Repository
rL LLVM

Event Timeline

spatel updated this revision to Diff 64381.Jul 18 2016, 2:18 PM
spatel retitled this revision from to [InstCombine] fold add(zext(xor X, C), C) --> sext X when C is INT_MIN in the source type.
spatel updated this object.
spatel added reviewers: majnemer, eli.friedman, sanjoy.
spatel added a subscriber: llvm-commits.
majnemer added inline comments.Jul 18 2016, 2:42 PM
lib/Transforms/InstCombine/InstCombineAddSub.cpp
1054 ↗(On Diff #64381)

Why m_OneUse ?

majnemer accepted this revision.Jul 18 2016, 2:43 PM
majnemer edited edge metadata.

The transform LGTM

This revision is now accepted and ready to land.Jul 18 2016, 2:43 PM
spatel added inline comments.Jul 18 2016, 2:51 PM
lib/Transforms/InstCombine/InstCombineAddSub.cpp
1054 ↗(On Diff #64381)

I was being conservative, but given that we'd at least shorten the dependency chain for this result regardless of other uses of the zext/xor, we don't need those checks.

I can remove the m_OneUse and add some more tests to this patch, or if you prefer, I'll add a TODO and follow-up patch.

majnemer added inline comments.Jul 18 2016, 2:55 PM
lib/Transforms/InstCombine/InstCombineAddSub.cpp
1054 ↗(On Diff #64381)

It seems much more canonical, I'd just remove the OneUse.

This revision was automatically updated to reflect the committed changes.