Skip to content

Commit e939e90

Browse files
jgu222igcbot
authored andcommitted
The existing I64 shift emu had wrong result if shift amt is not in [31, 0].
The problem is that the inner condition was generated incorrectly. This change fixes that.
1 parent 0dd6d9b commit e939e90

File tree

1 file changed

+17
-20
lines changed

1 file changed

+17
-20
lines changed

IGC/Compiler/CISACodeGen/Emu64OpsPass.cpp

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -794,13 +794,14 @@ bool InstExpander::visitShl(BinaryOperator& BinOp) {
794794

795795
Value* Cond = nullptr;
796796

797+
// Only LSB of ShAmt is used for shift
798+
ShAmt = IRB->CreateAnd(ShAmt, 63);
797799
if (!isa<ConstantInt>(ShAmt)) {
798800
// Create outer if-endif to handle the special case where `ShAmt` is zero.
799801
// We have option to handle that with `((S >> (~ShAmt)) >> 1)`. However, as
800802
// a zero `ShAmt` is a very rare case so that the outer branch should be
801803
// uniform one in most cases.
802-
Value* NE = IRB->CreateICmpNE(IRB->CreateAnd(ShAmt, 63),
803-
Constant::getNullValue(ShAmt->getType()));
804+
Value* NE = IRB->CreateICmpNE(ShAmt, Constant::getNullValue(ShAmt->getType()));
804805
BasicBlock* JointBB = OldBB->splitBasicBlock(&BinOp);
805806
ResLo = PHINode::Create(IRB->getInt32Ty(), 2, ".shl.outer.merge.lo", &BinOp);
806807
ResHi = PHINode::Create(IRB->getInt32Ty(), 2, ".shl.outer.merge.hi", &BinOp);
@@ -816,10 +817,9 @@ bool InstExpander::visitShl(BinaryOperator& BinOp) {
816817
// Create the inner branch.
817818
IRB->SetInsertPoint(&(*TrueBB->begin()));
818819

819-
Cond = IRB->CreateICmpEQ(IRB->CreateAnd(ShAmt, 32),
820-
Constant::getNullValue(ShAmt->getType()));
820+
Cond = IRB->CreateICmpULT(ShAmt, IRB->getInt32(32));
821821
// Prepare to generate branches to handle the case where `ShAmt` is less
822-
// than 32 or otherwise.
822+
// than 32 (true branch) or otherwise (false branch).
823823
BasicBlock* InnerJBB = TrueBB->splitBasicBlock(TrueJmp);
824824
InnerResLo = PHINode::Create(IRB->getInt32Ty(), 2, ".shl.merge.inner.lo", TrueJmp);
825825
InnerResHi = PHINode::Create(IRB->getInt32Ty(), 2, ".shl.merge.inner.hi", TrueJmp);
@@ -844,8 +844,7 @@ bool InstExpander::visitShl(BinaryOperator& BinOp) {
844844
cast<PHINode>(ResHi)->addIncoming(InnerResHi, InnerJBB);
845845
}
846846
else
847-
Cond = IRB->CreateICmpEQ(IRB->CreateAnd(ShAmt, 32),
848-
Constant::getNullValue(ShAmt->getType()));
847+
Cond = IRB->CreateICmpULT(ShAmt, IRB->getInt32(32));
849848

850849
if (Cond == IRB->getTrue() || InnerTBB) {
851850
if (InnerTBB) IRB->SetInsertPoint(&(*InnerTBB->begin()));
@@ -914,13 +913,14 @@ bool InstExpander::visitLShr(BinaryOperator& BinOp) {
914913

915914
Value* Cond = nullptr;
916915

916+
// Only LSB of ShAmt is used for shift
917+
ShAmt = IRB->CreateAnd(ShAmt, 63);
917918
if (!isa<ConstantInt>(ShAmt)) {
918919
// Create outer if-endif to handle the special case where `ShAmt` is zero.
919920
// We have option to handle that with `((S >> (~ShAmt)) >> 1)`. However, as
920921
// a zero `ShAmt` is a very rare case so that the outer branch should be
921922
// uniform one in most cases.
922-
Value* NE = IRB->CreateICmpNE(IRB->CreateAnd(ShAmt, 63),
923-
Constant::getNullValue(ShAmt->getType()));
923+
Value* NE = IRB->CreateICmpNE(ShAmt, Constant::getNullValue(ShAmt->getType()));
924924
BasicBlock* JointBB = OldBB->splitBasicBlock(&BinOp);
925925
ResLo = PHINode::Create(IRB->getInt32Ty(), 2, ".lshr.outer.merge.lo", &BinOp);
926926
ResHi = PHINode::Create(IRB->getInt32Ty(), 2, ".lshr.outer.merge.hi", &BinOp);
@@ -936,10 +936,9 @@ bool InstExpander::visitLShr(BinaryOperator& BinOp) {
936936
// Create the inner branch.
937937
IRB->SetInsertPoint(&(*TrueBB->begin()));
938938

939-
Cond = IRB->CreateICmpEQ(IRB->CreateAnd(ShAmt, 32),
940-
Constant::getNullValue(ShAmt->getType()));
939+
Cond = IRB->CreateICmpULT(ShAmt, IRB->getInt32(32));
941940
// Prepare to generate branches to handle the case where `ShAmt` is less
942-
// than 32 or otherwise.
941+
// than 32 (true branch) or otherwise (false branch).
943942
BasicBlock* InnerJBB = TrueBB->splitBasicBlock(TrueJmp);
944943
InnerResLo = PHINode::Create(IRB->getInt32Ty(), 2, ".lshr.merge.inner.lo", TrueJmp);
945944
InnerResHi = PHINode::Create(IRB->getInt32Ty(), 2, ".lshr.merge.inner.hi", TrueJmp);
@@ -964,8 +963,7 @@ bool InstExpander::visitLShr(BinaryOperator& BinOp) {
964963
cast<PHINode>(ResHi)->addIncoming(InnerResHi, InnerJBB);
965964
}
966965
else
967-
Cond = IRB->CreateICmpEQ(IRB->CreateAnd(ShAmt, 32),
968-
Constant::getNullValue(ShAmt->getType()));
966+
Cond = IRB->CreateICmpULT(ShAmt, IRB->getInt32(32));
969967

970968
if (Cond == IRB->getTrue() || InnerTBB) {
971969
if (InnerTBB) IRB->SetInsertPoint(&(*InnerTBB->begin()));
@@ -1034,13 +1032,14 @@ bool InstExpander::visitAShr(BinaryOperator& BinOp) {
10341032

10351033
Value* Cond = nullptr;
10361034

1035+
// Only LSB of ShAmt is used for shift
1036+
ShAmt = IRB->CreateAnd(ShAmt, 63);
10371037
if (!isa<ConstantInt>(ShAmt)) {
10381038
// Create outer if-endif to handle the special case where `ShAmt` is zero.
10391039
// We have option to handle that with `((S >> (~ShAmt)) >> 1)`. However, as
10401040
// a zero `ShAmt` is a very rare case so that the outer branch should be
10411041
// uniform one in most cases.
1042-
Value* NE = IRB->CreateICmpNE(IRB->CreateAnd(ShAmt, 63),
1043-
Constant::getNullValue(ShAmt->getType()));
1042+
Value* NE = IRB->CreateICmpNE(ShAmt, Constant::getNullValue(ShAmt->getType()));
10441043
BasicBlock* JointBB = OldBB->splitBasicBlock(&BinOp);
10451044
ResLo = PHINode::Create(IRB->getInt32Ty(), 2, ".ashr.outer.merge.lo", &BinOp);
10461045
ResHi = PHINode::Create(IRB->getInt32Ty(), 2, ".ashr.outer.merge.hi", &BinOp);
@@ -1056,8 +1055,7 @@ bool InstExpander::visitAShr(BinaryOperator& BinOp) {
10561055
// Create the inner branch.
10571056
IRB->SetInsertPoint(&(*TrueBB->begin()));
10581057

1059-
Cond = IRB->CreateICmpEQ(IRB->CreateAnd(ShAmt, 32),
1060-
Constant::getNullValue(ShAmt->getType()));
1058+
Cond = IRB->CreateICmpULT(ShAmt, IRB->getInt32(32));
10611059
// Prepare to generate branches to handle the case where `ShAmt` is less
10621060
// than 32 or otherwise.
10631061
BasicBlock* InnerJBB = TrueBB->splitBasicBlock(TrueJmp);
@@ -1084,8 +1082,7 @@ bool InstExpander::visitAShr(BinaryOperator& BinOp) {
10841082
cast<PHINode>(ResHi)->addIncoming(InnerResHi, InnerJBB);
10851083
}
10861084
else
1087-
Cond = IRB->CreateICmpEQ(IRB->CreateAnd(ShAmt, 32),
1088-
Constant::getNullValue(ShAmt->getType()));
1085+
Cond = IRB->CreateICmpULT(ShAmt, IRB->getInt32(32));
10891086

10901087
if (Cond == IRB->getTrue() || InnerTBB) {
10911088
if (InnerTBB) IRB->SetInsertPoint(&(*InnerTBB->begin()));

0 commit comments

Comments
 (0)