This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] (1 << (C - x)) -> ((1 << C) >> x) if C is bitwidth - 1
ClosedPublic

Authored by xbolva00 on Jun 21 2019, 8:18 AM.

Details

Diff Detail

Repository
rL LLVM

Event Timeline

xbolva00 created this revision.Jun 21 2019, 8:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 21 2019, 8:18 AM
xbolva00 updated this revision to Diff 206005.Jun 21 2019, 8:20 AM
nikic added inline comments.Jun 21 2019, 8:37 AM
lib/Transforms/InstCombine/InstCombineShifts.cpp
677 ↗(On Diff #206005)

getSExtValue will assert if the value is too large. You can use ShlOp0->isOneValue() here.

679 ↗(On Diff #206005)

Same issue here. I think just doing *C == BitWidth - 1 should work.

lebedev.ri added inline comments.Jun 21 2019, 8:39 AM
lib/Transforms/InstCombine/InstCombineShifts.cpp
677 ↗(On Diff #206005)

m_One()

678–679 ↗(On Diff #206005)

m_SpecificInt(BitWidth - 1)

680 ↗(On Diff #206005)

ConstantInt::get(Ty, APInt::getSignMask(BitWidth))

nikic added inline comments.Jun 21 2019, 8:40 AM
lib/Transforms/InstCombine/InstCombineShifts.cpp
677 ↗(On Diff #206005)

Or maybe even simpler, use match(Op0, m_One()).

xbolva00 updated this revision to Diff 206010.Jun 21 2019, 8:46 AM

Simplified code.

Thanks for cool notes, @nikic, @lebedev.ri !

Please mark fixed comments as such

xbolva00 marked 5 inline comments as done.Jun 21 2019, 8:55 AM
lebedev.ri accepted this revision.Jun 21 2019, 8:58 AM

Please precommit those 3 extra tests first though

test/Transforms/InstCombine/shl-sub.ll
56 ↗(On Diff #206010)

missing 3 tests with undef:

define <3 x i64> @shl_sub_i64_vec_undef0(<3 x i64> %x) {
  %s = sub <3 x i64> <i64 63, i64 undef, i64 63>, %x
  %r = shl <3 x i64> <i64 1, i64 1, i64 1>, %s
  ret <3 x i64> %r
}

define <3 x i64> @shl_sub_i64_vec_undef1(<3 x i64> %x) {
  %s = sub <3 x i64> <i64 63, undef, i64 63>, %x
  %r = shl <3 x i64> <i64 1, i64 undef, i64 1>, %s
  ret <3 x i64> %r
}

define <3 x i64> @shl_sub_i64_vec_undef2(<3 x i64> %x) {
  %s = sub <3 x i64> <i64 63, i64 undef, i64 63>, %x
  %r = shl <3 x i64> <i64 1, i64 undef, i64 1>, %s
  ret <3 x i64> %r
}
This revision is now accepted and ready to land.Jun 21 2019, 8:58 AM
xbolva00 updated this revision to Diff 206015.Jun 21 2019, 9:12 AM

Updated tests

This revision was automatically updated to reflect the committed changes.