Skip to content

Commit 559d966

Browse files
yingopqdyung
yingopq
authored and
dyung
committed
[Mips] Fix atomic min/max generate mips4 instructions when compiling for mips2 (llvm#159717)
Modify instr movn/movz to mixture of beq, move, and sc. Because atomic-min-max.ll test broken on the expensive builder, I revert llvm#149983 and resubmit this PR. The broken reason: In i16/i8 function expandAtomicBinOpSubword, we use two successor after loop2MBB, one does not specify the second parameter, the other use BranchProbability::getOne() that means 100% probability. This is contradictory. And the second successor is also specified incorrectly. The changess: * llvm/lib/Target/Mips/MipsExpandPseudo.cpp: Change loop2MBB`s second successor to correct one and delete the second parameter BranchProbability::getOne(). * llvm/test/CodeGen/Mips/atomic-min-max.ll: Add -verify-machineinstrs option in RUN command; Modify i16 test and i8 test according to the changes. Fix llvm#145411. (cherry picked from commit 114b3b8)
1 parent d1e2f89 commit 559d966

File tree

2 files changed

+712
-26
lines changed

2 files changed

+712
-26
lines changed

‎llvm/lib/Target/Mips/MipsExpandPseudo.cpp‎

Lines changed: 191 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -432,23 +432,44 @@ bool MipsExpandPseudo::expandAtomicBinOpSubword(
432432
Register OldVal = I->getOperand(6).getReg();
433433
Register BinOpRes = I->getOperand(7).getReg();
434434
Register StoreVal = I->getOperand(8).getReg();
435+
bool NoMovnInstr = (IsMin || IsMax) && !STI->hasMips4() && !STI->hasMips32();
435436

436437
const BasicBlock *LLVM_BB = BB.getBasicBlock();
437438
MachineBasicBlock *loopMBB = MF->CreateMachineBasicBlock(LLVM_BB);
439+
MachineBasicBlock *loop1MBB = nullptr;
440+
MachineBasicBlock *loop2MBB = nullptr;
441+
if (NoMovnInstr) {
442+
loop1MBB = MF->CreateMachineBasicBlock(LLVM_BB);
443+
loop2MBB = MF->CreateMachineBasicBlock(LLVM_BB);
444+
}
438445
MachineBasicBlock *sinkMBB = MF->CreateMachineBasicBlock(LLVM_BB);
439446
MachineBasicBlock *exitMBB = MF->CreateMachineBasicBlock(LLVM_BB);
440447
MachineFunction::iterator It = ++BB.getIterator();
441448
MF->insert(It, loopMBB);
449+
if (NoMovnInstr) {
450+
MF->insert(It, loop1MBB);
451+
MF->insert(It, loop2MBB);
452+
}
442453
MF->insert(It, sinkMBB);
443454
MF->insert(It, exitMBB);
444455

445456
exitMBB->splice(exitMBB->begin(), &BB, std::next(I), BB.end());
446457
exitMBB->transferSuccessorsAndUpdatePHIs(&BB);
447458

448459
BB.addSuccessor(loopMBB, BranchProbability::getOne());
449-
loopMBB->addSuccessor(sinkMBB);
450-
loopMBB->addSuccessor(loopMBB);
451-
loopMBB->normalizeSuccProbs();
460+
if (NoMovnInstr) {
461+
loopMBB->addSuccessor(loop1MBB);
462+
loopMBB->addSuccessor(loop2MBB);
463+
} else {
464+
loopMBB->addSuccessor(sinkMBB);
465+
loopMBB->addSuccessor(loopMBB);
466+
loopMBB->normalizeSuccProbs();
467+
}
468+
if (NoMovnInstr) {
469+
loop1MBB->addSuccessor(loop2MBB);
470+
loop2MBB->addSuccessor(loopMBB);
471+
loop2MBB->addSuccessor(sinkMBB);
472+
}
452473

453474
BuildMI(loopMBB, DL, TII->get(LL), OldVal).addReg(Ptr).addImm(0);
454475
if (IsNand) {
@@ -525,7 +546,7 @@ bool MipsExpandPseudo::expandAtomicBinOpSubword(
525546
BuildMI(loopMBB, DL, TII->get(OR), BinOpRes)
526547
.addReg(BinOpRes)
527548
.addReg(Scratch4);
528-
} else {
549+
} else if (STI->hasMips4() || STI->hasMips32()) {
529550
// max: move BinOpRes, StoreVal
530551
// movn BinOpRes, Incr, Scratch4, BinOpRes
531552
// min: move BinOpRes, StoreVal
@@ -537,12 +558,59 @@ bool MipsExpandPseudo::expandAtomicBinOpSubword(
537558
.addReg(Incr)
538559
.addReg(Scratch4)
539560
.addReg(BinOpRes);
561+
} else {
562+
// if min:
563+
// loopMBB: move BinOpRes, StoreVal
564+
// beq Scratch4, 0, loop1MBB
565+
// j loop2MBB
566+
// loop1MBB: move BinOpRes, Incr
567+
// loop2MBB: and BinOpRes, BinOpRes, Mask
568+
// and StoreVal, OlddVal, Mask2
569+
// or StoreVal, StoreVal, BinOpRes
570+
// StoreVal<tied1> = sc StoreVal, 0(Ptr)
571+
// beq StoreVal, zero, loopMBB
572+
//
573+
// if max:
574+
// loopMBB: move BinOpRes, Incr
575+
// beq Scratch4, 0, loop1MBB
576+
// j loop2MBB
577+
// loop1MBB: move BinOpRes, StoreVal
578+
// loop2MBB: and BinOpRes, BinOpRes, Mask
579+
// and StoreVal, OlddVal, Mask2
580+
// or StoreVal, StoreVal, BinOpRes
581+
// StoreVal<tied1> = sc StoreVal, 0(Ptr)
582+
// beq StoreVal, zero, loopMBB
583+
if (IsMin) {
584+
BuildMI(loopMBB, DL, TII->get(OR), BinOpRes)
585+
.addReg(StoreVal)
586+
.addReg(Mips::ZERO);
587+
BuildMI(loop1MBB, DL, TII->get(OR), BinOpRes)
588+
.addReg(Incr)
589+
.addReg(Mips::ZERO);
590+
} else {
591+
BuildMI(loopMBB, DL, TII->get(OR), BinOpRes)
592+
.addReg(Incr)
593+
.addReg(Mips::ZERO);
594+
BuildMI(loop1MBB, DL, TII->get(OR), BinOpRes)
595+
.addReg(StoreVal)
596+
.addReg(Mips::ZERO);
597+
}
598+
BuildMI(loopMBB, DL, TII->get(BEQ))
599+
.addReg(Scratch4)
600+
.addReg(Mips::ZERO)
601+
.addMBB(loop1MBB);
602+
BuildMI(loopMBB, DL, TII->get(Mips::J)).addMBB(loop2MBB);
540603
}
541604

542605
// and BinOpRes, BinOpRes, Mask
543-
BuildMI(loopMBB, DL, TII->get(Mips::AND), BinOpRes)
544-
.addReg(BinOpRes)
545-
.addReg(Mask);
606+
if (NoMovnInstr)
607+
BuildMI(loop2MBB, DL, TII->get(Mips::AND), BinOpRes)
608+
.addReg(BinOpRes)
609+
.addReg(Mask);
610+
else
611+
BuildMI(loopMBB, DL, TII->get(Mips::AND), BinOpRes)
612+
.addReg(BinOpRes)
613+
.addReg(Mask);
546614

547615
} else if (!IsSwap) {
548616
// <binop> binopres, oldval, incr2
@@ -564,14 +632,37 @@ bool MipsExpandPseudo::expandAtomicBinOpSubword(
564632
// or StoreVal, StoreVal, BinOpRes
565633
// StoreVal<tied1> = sc StoreVal, 0(Ptr)
566634
// beq StoreVal, zero, loopMBB
567-
BuildMI(loopMBB, DL, TII->get(Mips::AND), StoreVal)
568-
.addReg(OldVal).addReg(Mask2);
569-
BuildMI(loopMBB, DL, TII->get(Mips::OR), StoreVal)
570-
.addReg(StoreVal).addReg(BinOpRes);
571-
BuildMI(loopMBB, DL, TII->get(SC), StoreVal)
572-
.addReg(StoreVal).addReg(Ptr).addImm(0);
573-
BuildMI(loopMBB, DL, TII->get(BEQ))
574-
.addReg(StoreVal).addReg(Mips::ZERO).addMBB(loopMBB);
635+
if (NoMovnInstr) {
636+
BuildMI(loop2MBB, DL, TII->get(Mips::AND), StoreVal)
637+
.addReg(OldVal)
638+
.addReg(Mask2);
639+
BuildMI(loop2MBB, DL, TII->get(Mips::OR), StoreVal)
640+
.addReg(StoreVal)
641+
.addReg(BinOpRes);
642+
BuildMI(loop2MBB, DL, TII->get(SC), StoreVal)
643+
.addReg(StoreVal)
644+
.addReg(Ptr)
645+
.addImm(0);
646+
BuildMI(loop2MBB, DL, TII->get(BEQ))
647+
.addReg(StoreVal)
648+
.addReg(Mips::ZERO)
649+
.addMBB(loopMBB);
650+
} else {
651+
BuildMI(loopMBB, DL, TII->get(Mips::AND), StoreVal)
652+
.addReg(OldVal)
653+
.addReg(Mask2);
654+
BuildMI(loopMBB, DL, TII->get(Mips::OR), StoreVal)
655+
.addReg(StoreVal)
656+
.addReg(BinOpRes);
657+
BuildMI(loopMBB, DL, TII->get(SC), StoreVal)
658+
.addReg(StoreVal)
659+
.addReg(Ptr)
660+
.addImm(0);
661+
BuildMI(loopMBB, DL, TII->get(BEQ))
662+
.addReg(StoreVal)
663+
.addReg(Mips::ZERO)
664+
.addMBB(loopMBB);
665+
}
575666

576667
// sinkMBB:
577668
// and maskedoldval1,oldval,mask
@@ -600,6 +691,11 @@ bool MipsExpandPseudo::expandAtomicBinOpSubword(
600691

601692
LivePhysRegs LiveRegs;
602693
computeAndAddLiveIns(LiveRegs, *loopMBB);
694+
if (loop1MBB) {
695+
assert(loop2MBB && "should have 2 loop blocks");
696+
computeAndAddLiveIns(LiveRegs, *loop1MBB);
697+
computeAndAddLiveIns(LiveRegs, *loop2MBB);
698+
}
603699
computeAndAddLiveIns(LiveRegs, *sinkMBB);
604700
computeAndAddLiveIns(LiveRegs, *exitMBB);
605701

@@ -746,20 +842,41 @@ bool MipsExpandPseudo::expandAtomicBinOp(MachineBasicBlock &BB,
746842
llvm_unreachable("Unknown pseudo atomic!");
747843
}
748844

845+
bool NoMovnInstr = (IsMin || IsMax) && !STI->hasMips4() && !STI->hasMips32();
749846
const BasicBlock *LLVM_BB = BB.getBasicBlock();
750847
MachineBasicBlock *loopMBB = MF->CreateMachineBasicBlock(LLVM_BB);
848+
MachineBasicBlock *loop1MBB = nullptr;
849+
MachineBasicBlock *loop2MBB = nullptr;
850+
if (NoMovnInstr) {
851+
loop1MBB = MF->CreateMachineBasicBlock(LLVM_BB);
852+
loop2MBB = MF->CreateMachineBasicBlock(LLVM_BB);
853+
}
751854
MachineBasicBlock *exitMBB = MF->CreateMachineBasicBlock(LLVM_BB);
752855
MachineFunction::iterator It = ++BB.getIterator();
753856
MF->insert(It, loopMBB);
857+
if (NoMovnInstr) {
858+
MF->insert(It, loop1MBB);
859+
MF->insert(It, loop2MBB);
860+
}
754861
MF->insert(It, exitMBB);
755862

756863
exitMBB->splice(exitMBB->begin(), &BB, std::next(I), BB.end());
757864
exitMBB->transferSuccessorsAndUpdatePHIs(&BB);
758865

759866
BB.addSuccessor(loopMBB, BranchProbability::getOne());
760-
loopMBB->addSuccessor(exitMBB);
761-
loopMBB->addSuccessor(loopMBB);
867+
if (NoMovnInstr) {
868+
loopMBB->addSuccessor(loop1MBB);
869+
loopMBB->addSuccessor(loop2MBB);
870+
} else {
871+
loopMBB->addSuccessor(exitMBB);
872+
loopMBB->addSuccessor(loopMBB);
873+
}
762874
loopMBB->normalizeSuccProbs();
875+
if (NoMovnInstr) {
876+
loop1MBB->addSuccessor(loop2MBB);
877+
loop2MBB->addSuccessor(loopMBB);
878+
loop2MBB->addSuccessor(exitMBB);
879+
}
763880

764881
BuildMI(loopMBB, DL, TII->get(LL), OldVal).addReg(Ptr).addImm(0);
765882
assert((OldVal != Ptr) && "Clobbered the wrong ptr reg!");
@@ -802,7 +919,7 @@ bool MipsExpandPseudo::expandAtomicBinOp(MachineBasicBlock &BB,
802919
BuildMI(loopMBB, DL, TII->get(OR), Scratch)
803920
.addReg(Scratch)
804921
.addReg(Scratch2);
805-
} else {
922+
} else if (STI->hasMips4() || STI->hasMips32()) {
806923
// max: move Scratch, OldVal
807924
// movn Scratch, Incr, Scratch2, Scratch
808925
// min: move Scratch, OldVal
@@ -814,6 +931,38 @@ bool MipsExpandPseudo::expandAtomicBinOp(MachineBasicBlock &BB,
814931
.addReg(Incr)
815932
.addReg(Scratch2)
816933
.addReg(Scratch);
934+
} else {
935+
// if min:
936+
// loopMBB: move Scratch, OldVal
937+
// beq Scratch2_32, 0, loop1MBB
938+
// j loop2MBB
939+
// loop1MBB: move Scratch, Incr
940+
// loop2MBB: sc $2, 0($4)
941+
// beqz $2, $BB0_1
942+
// nop
943+
//
944+
// if max:
945+
// loopMBB: move Scratch, Incr
946+
// beq Scratch2_32, 0, loop1MBB
947+
// j loop2MBB
948+
// loop1MBB: move Scratch, OldVal
949+
// loop2MBB: sc $2, 0($4)
950+
// beqz $2, $BB0_1
951+
// nop
952+
if (IsMin) {
953+
BuildMI(loopMBB, DL, TII->get(OR), Scratch).addReg(OldVal).addReg(ZERO);
954+
BuildMI(loop1MBB, DL, TII->get(OR), Scratch).addReg(Incr).addReg(ZERO);
955+
} else {
956+
BuildMI(loopMBB, DL, TII->get(OR), Scratch).addReg(Incr).addReg(ZERO);
957+
BuildMI(loop1MBB, DL, TII->get(OR), Scratch)
958+
.addReg(OldVal)
959+
.addReg(ZERO);
960+
}
961+
BuildMI(loopMBB, DL, TII->get(BEQ))
962+
.addReg(Scratch2_32)
963+
.addReg(ZERO)
964+
.addMBB(loop1MBB);
965+
BuildMI(loopMBB, DL, TII->get(Mips::J)).addMBB(loop2MBB);
817966
}
818967

819968
} else if (Opcode) {
@@ -829,20 +978,36 @@ bool MipsExpandPseudo::expandAtomicBinOp(MachineBasicBlock &BB,
829978
BuildMI(loopMBB, DL, TII->get(OR), Scratch).addReg(Incr).addReg(ZERO);
830979
}
831980

832-
BuildMI(loopMBB, DL, TII->get(SC), Scratch)
833-
.addReg(Scratch)
834-
.addReg(Ptr)
835-
.addImm(0);
836-
BuildMI(loopMBB, DL, TII->get(BEQ))
837-
.addReg(Scratch)
838-
.addReg(ZERO)
839-
.addMBB(loopMBB);
981+
if (NoMovnInstr) {
982+
BuildMI(loop2MBB, DL, TII->get(SC), Scratch)
983+
.addReg(Scratch)
984+
.addReg(Ptr)
985+
.addImm(0);
986+
BuildMI(loop2MBB, DL, TII->get(BEQ))
987+
.addReg(Scratch)
988+
.addReg(ZERO)
989+
.addMBB(loopMBB);
990+
} else {
991+
BuildMI(loopMBB, DL, TII->get(SC), Scratch)
992+
.addReg(Scratch)
993+
.addReg(Ptr)
994+
.addImm(0);
995+
BuildMI(loopMBB, DL, TII->get(BEQ))
996+
.addReg(Scratch)
997+
.addReg(ZERO)
998+
.addMBB(loopMBB);
999+
}
8401000

8411001
NMBBI = BB.end();
8421002
I->eraseFromParent();
8431003

8441004
LivePhysRegs LiveRegs;
8451005
computeAndAddLiveIns(LiveRegs, *loopMBB);
1006+
if (loop1MBB) {
1007+
assert(loop2MBB && "should have 2 loop blocks");
1008+
computeAndAddLiveIns(LiveRegs, *loop1MBB);
1009+
computeAndAddLiveIns(LiveRegs, *loop2MBB);
1010+
}
8461011
computeAndAddLiveIns(LiveRegs, *exitMBB);
8471012

8481013
return true;

0 commit comments

Comments
 (0)