Skip to content

Commit 45abde9

Browse files
committed
wip: I fixed all issues reported with -verify-machineinstrs, need LARGE cleanup now ...
While at it, I implemented some quick optimization. - Implemented `(1 << n) - 1` == `lslx lneg n` #9 - Moved some 64-bit operation earlier in the pipeline, from ResolveMacroInstrPass to expandPostRA. I tried even earlier, but SUBC is not well defined and get moved around during MergeSink for critical edge split. The computation is then wrong because require to be packed for the particular use I identified. I leave my experiment here. I will check to fully define it and move it preRA, or will leave it like that again for a while, fulfilling my other duty. - Fixed a few easy Def/Use when BuildMI - Fixed lose of MachineInstr correctness Our arithmetic+comp+branch was destroyed during analyzeBranch/removeBranch/insertBranch - temporarily removed fusion of any instruction + JUMPi in MergeComboInstr The problem is that at this stage (PreEmit): - machine CFG is done. - JUMPi is unconditional jump - arithmetic + cond + branch; with cond as True/False is conditional -- even if we know that cond with true/false is unconditional, the instruction have the -- property of being conditional by its definition. ---- To fix that, I will create other PseudoInstruction to have them set correctly. ---- Also, if those arith+cond+branch do have pattern, maybe they could be selected far earlier ---- and the machine CFG would be correctly formed at the first place probably. - issue with ThinLTO fixed -- some code construction ended up in SELECTrr, which is not common for us ---- this is lowered to TmpJcc ---- and TmpJcc is kind of wrong ------ and finally, MergeComboInstr was combining even more wrong. --> I removed TmpJcc, and use simply our well defined JEQrii - issue whith ThinLTO fixed -- another was present but undetected on Release build -- we use multiple address spacees (IRAM, WRAM, MRAM) -- there was an assertion with ThinLTO when populating GV out of multiple modules ---- it's fixed in llvm13, but we are on llvm12 ------ I reproduced the patch (not cherry-picked) llvm@60c60dd ------ just for now. will do that correctly when cleaning up ------- so when I will upgrade our LLVM, it will be mergeable easily
1 parent bbb5895 commit 45abde9

15 files changed

+1716
-863
lines changed

llvm/lib/CodeGen/MachineSink.cpp

Lines changed: 46 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,23 +1103,51 @@ bool MachineSinking::hasStoreBetween(MachineBasicBlock *From,
11031103
bool MachineSinking::SinkInstruction(MachineInstr &MI, bool &SawStore,
11041104
AllSuccsCache &AllSuccessors) {
11051105
// Don't sink instructions that the target prefers not to sink.
1106-
if (!TII->shouldSink(MI))
1106+
if (!TII->shouldSink(MI)) {
1107+
// LLVM_DEBUG({
1108+
// dbgs() << "shouldSink false "; MI.dump();
1109+
// });
11071110
return false;
1108-
1111+
}
1112+
11091113
// Check if it's safe to move the instruction.
1110-
if (!MI.isSafeToMove(AA, SawStore))
1114+
if (!MI.isSafeToMove(AA, SawStore)) {
1115+
// LLVM_DEBUG({
1116+
// dbgs() << "not safe "; MI.dump();
1117+
// dbgs() << "mayStore(): " << MI.mayStore() << "\n";
1118+
// dbgs() << "mayLoad(): " << MI.mayLoad() << "\n";
1119+
// dbgs() << "isCall(): " << MI.isCall() << "\n";
1120+
// dbgs() << "isPHI(): " << MI.isPHI() << "\n";
1121+
// dbgs() << "hasOrderedMemoryRef(): " << MI.hasOrderedMemoryRef() << "\n";
1122+
// dbgs() << "isPosition(): " << MI.isPosition() << "\n";
1123+
// dbgs() << "isDebugInstr(): " << MI.isDebugInstr() << "\n";
1124+
// dbgs() << "isTerminator(): " << MI.isTerminator() << "\n";
1125+
// dbgs() << "mayRaiseFPException(): " << MI.mayRaiseFPException() << "\n";
1126+
// dbgs() << "hasUnmodeledSideEffects(): " << MI.hasUnmodeledSideEffects() << "\n";
1127+
// dbgs() << "isDereferenceableInvariantLoad(AA): " << MI.isDereferenceableInvariantLoad(AA) << "\n";
1128+
// dbgs() << "SawStore: " << SawStore << "\n";
1129+
// });
11111130
return false;
1112-
1131+
}
1132+
11131133
// Convergent operations may not be made control-dependent on additional
11141134
// values.
1115-
if (MI.isConvergent())
1135+
if (MI.isConvergent()) {
1136+
// LLVM_DEBUG({
1137+
// dbgs() << "isconvergent "; MI.dump();
1138+
// });
11161139
return false;
1117-
1140+
}
1141+
11181142
// Don't break implicit null checks. This is a performance heuristic, and not
11191143
// required for correctness.
1120-
if (SinkingPreventsImplicitNullCheck(MI, TII, TRI))
1144+
if (SinkingPreventsImplicitNullCheck(MI, TII, TRI)) {
1145+
LLVM_DEBUG({
1146+
dbgs() << "nullcheck "; MI.dump();
1147+
});
11211148
return false;
1122-
1149+
}
1150+
11231151
// FIXME: This should include support for sinking instructions within the
11241152
// block they are currently in to shorten the live ranges. We often get
11251153
// instructions sunk into the top of a large block, but it would be better to
@@ -1134,9 +1162,12 @@ bool MachineSinking::SinkInstruction(MachineInstr &MI, bool &SawStore,
11341162
FindSuccToSinkTo(MI, ParentBlock, BreakPHIEdge, AllSuccessors);
11351163

11361164
// If there are no outputs, it must have side-effects.
1137-
if (!SuccToSinkTo)
1165+
if (!SuccToSinkTo) {
1166+
// LLVM_DEBUG({
1167+
// dbgs() << "no succ "; MI.dump();
1168+
// });
11381169
return false;
1139-
1170+
}
11401171
// If the instruction to move defines a dead physical register which is live
11411172
// when leaving the basic block, don't move it because it could turn into a
11421173
// "zombie" define of that preg. E.g., EFLAGS. (<rdar://problem/8030636>)
@@ -1146,8 +1177,12 @@ bool MachineSinking::SinkInstruction(MachineInstr &MI, bool &SawStore,
11461177
Register Reg = MO.getReg();
11471178
if (Reg == 0 || !Register::isPhysicalRegister(Reg))
11481179
continue;
1149-
if (SuccToSinkTo->isLiveIn(Reg))
1180+
if (SuccToSinkTo->isLiveIn(Reg)) {
1181+
// LLVM_DEBUG({
1182+
// dbgs() << "zombie "; MI.dump();
1183+
// });
11501184
return false;
1185+
}
11511186
}
11521187

11531188
LLVM_DEBUG(dbgs() << "Sink instr " << MI << "\tinto block " << *SuccToSinkTo);

0 commit comments

Comments
 (0)