Page MenuHomePhabricator

[InstCombine] Catch more bswap cases missed due to zext and truncs.
ClosedPublic

Authored by mcrosier on May 24 2016, 12:35 PM.

Details

Summary

This patch addresses PR27824: https://llvm.org/bugs/show_bug.cgi?id=27824

On AArch64 the i16 type is not legal type, so we end up with IR like what is included in the test cases.

Please take a look,
Chad

Diff Detail

Event Timeline

mcrosier updated this revision to Diff 58287.May 24 2016, 12:35 PM
mcrosier retitled this revision from to [InstCombine] Catch more bswap cases missed due to zext and truncs..
mcrosier updated this object.
mcrosier added a subscriber: llvm-commits.
gberry added inline comments.May 25 2016, 10:40 AM
test/Transforms/InstCombine/bswap.ll
106

Shouldn't this be an i32 lshr of %conv and the second zext below deleted if you want the IR to look like it would for a target without i16 as a legal type?

jmolloy accepted this revision.May 26 2016, 2:52 AM
jmolloy edited edge metadata.

LGTM!

This revision is now accepted and ready to land.May 26 2016, 2:52 AM
mcrosier added inline comments.May 26 2016, 6:40 AM
test/Transforms/InstCombine/bswap.ll
106

With the exception of relabeling the variables and dropping irrelevant nsw/nuws, this is the code that is generated from this C test case when targeting AArch64:

unsigned short test16(unsigned short a) {
  unsigned short b = (a & 0xff00) >> 8;
  unsigned short c = (a & 0x00ff) << 8;
  return b | c;
}

Produces:

define i16 @test16(i16 %a) #0 {
entry:
  %conv = zext i16 %a to i32
  %shr11 = lshr i16 %a, 8
  %and3 = shl nuw nsw i32 %conv, 8
  %conv5 = zext i16 %shr11 to i32
  %or = or i32 %conv5, %and3
  %conv7 = trunc i32 %or to i16
  ret i16 %conv7
}

Regardless, I'm happy to add a test case similar to what you're suggesting.

mcrosier closed this revision.May 26 2016, 8:32 AM

Committed in r270853.