Skip to content

Commit

Permalink
[InstCombine] Add single use checks to SimplifyBSwap to ensure we are…
Browse files Browse the repository at this point in the history
… really saving instructions

Bswap isn't a simple operation so we need to make sure we are really removing a call to it before doing these simplifications.

For the case when both LHS and RHS are bswaps I've allowed it to be moved if either LHS or RHS has a single use since that at least allows us to move it later where it might find another bswap to combine with and it decreases the use count on the other side so maybe the other user can be optimized.

Differential Revision: https://reviews.llvm.org/D34974

llvm-svn: 307273
  • Loading branch information
topperc committed Jul 6, 2017
1 parent f0f2960 commit 22795de
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 10 deletions.
13 changes: 9 additions & 4 deletions llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
Expand Up @@ -82,20 +82,25 @@ static Value *getFCmpValue(unsigned Code, Value *LHS, Value *RHS,
Value *InstCombiner::SimplifyBSwap(BinaryOperator &I) {
assert(I.isBitwiseLogicOp() && "Unexpected opcode for bswap simplifying");

// TODO We should probably check for single use of the bswap.
Value *OldLHS = I.getOperand(0);
Value *OldRHS = I.getOperand(1);

Value *NewLHS;
if (!match(I.getOperand(0), m_BSwap(m_Value(NewLHS))))
if (!match(OldLHS, m_BSwap(m_Value(NewLHS))))
return nullptr;

Value *NewRHS;
const APInt *C;

if (match(I.getOperand(1), m_BSwap(m_Value(NewRHS)))) {
if (match(OldRHS, m_BSwap(m_Value(NewRHS)))) {
// OP( BSWAP(x), BSWAP(y) ) -> BSWAP( OP(x, y) )
if (!OldLHS->hasOneUse() && !OldRHS->hasOneUse())
return nullptr;
// NewRHS initialized by the matcher.
} else if (match(I.getOperand(1), m_APInt(C))) {
} else if (match(OldRHS, m_APInt(C))) {
// OP( BSWAP(x), CONSTANT ) -> BSWAP( OP(x, BSWAP(CONSTANT) ) )
if (!OldLHS->hasOneUse())
return nullptr;
NewRHS = ConstantInt::get(I.getType(), C->byteSwap());
} else
return nullptr;
Expand Down
10 changes: 4 additions & 6 deletions llvm/test/Transforms/InstCombine/bswap-fold.ll
Expand Up @@ -285,9 +285,8 @@ define i64 @bs_and64_multiuse1(i64 %a, i64 %b) #0 {
; CHECK-LABEL: @bs_and64_multiuse1(
; CHECK-NEXT: [[TMP1:%.*]] = tail call i64 @llvm.bswap.i64(i64 [[A:%.*]])
; CHECK-NEXT: [[TMP2:%.*]] = tail call i64 @llvm.bswap.i64(i64 [[B:%.*]])
; CHECK-NEXT: [[TMP3:%.*]] = and i64 [[A]], [[B]]
; CHECK-NEXT: [[TMP6:%.*]] = call i64 @llvm.bswap.i64(i64 [[TMP3]])
; CHECK-NEXT: [[TMP4:%.*]] = mul i64 [[TMP6]], [[TMP1]]
; CHECK-NEXT: [[TMP3:%.*]] = and i64 [[TMP1]], [[TMP2]]
; CHECK-NEXT: [[TMP4:%.*]] = mul i64 [[TMP3]], [[TMP1]]
; CHECK-NEXT: [[TMP5:%.*]] = mul i64 [[TMP4]], [[TMP2]]
; CHECK-NEXT: ret i64 [[TMP5]]
;
Expand Down Expand Up @@ -332,9 +331,8 @@ define i64 @bs_and64_multiuse3(i64 %a, i64 %b) #0 {
define i64 @bs_and64i_multiuse(i64 %a, i64 %b) #0 {
; CHECK-LABEL: @bs_and64i_multiuse(
; CHECK-NEXT: [[TMP1:%.*]] = tail call i64 @llvm.bswap.i64(i64 [[A:%.*]])
; CHECK-NEXT: [[TMP2:%.*]] = and i64 [[A]], 129085117527228416
; CHECK-NEXT: [[TMP4:%.*]] = call i64 @llvm.bswap.i64(i64 [[TMP2]])
; CHECK-NEXT: [[TMP3:%.*]] = mul i64 [[TMP4]], [[TMP1]]
; CHECK-NEXT: [[TMP2:%.*]] = and i64 [[TMP1]], 1000000001
; CHECK-NEXT: [[TMP3:%.*]] = mul i64 [[TMP2]], [[TMP1]]
; CHECK-NEXT: ret i64 [[TMP3]]
;
%tmp1 = tail call i64 @llvm.bswap.i64(i64 %a)
Expand Down

0 comments on commit 22795de

Please sign in to comment.