This is an archive of the discontinued LLVM Phabricator instance.

Differential D105684

[RegisterCoalescer] Make resolveConflicts aware of earlyclobber
ClosedPublic

Authored by arcbbb on Jul 9 2021, 2:07 AM.

Details

Reviewers
stoklund
qcolombet
craig.topper
HsiangKai
Commits
rG8d86562e5f1f: [RegisterCoalescer] Make resolveConflicts aware of earlyclobber
Summary

Prior to this patch, it skipped the instruction defining VNI when checking if the tainted lanes are used.
In the given example, VRGATHER is an illegal instruction because its DstReg overlaps with SrcReg.

Therefore we need to check the defining instruction as well when there is an earlyclobber constraint.

Diff Detail

Event Timeline

arcbbb created this revision.Jul 9 2021, 2:07 AM
Herald added subscribers: tpr, hiraditya, qcolombet, MatzeB. · View Herald TranscriptJul 9 2021, 2:07 AM
arcbbb requested review of this revision.Jul 9 2021, 2:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 9 2021, 2:07 AM
Herald added a subscriber: llvm-commits. · View Herald Transcript
arcbbb added a parent revision: D105685: [RISCV][RVV] Precommit a test case for D105684.Jul 9 2021, 2:12 AM
arcbbb updated this revision to Diff 357448.Jul 9 2021, 2:20 AM
arcbbb edited the summary of this revision. (Show Details)

update a test case to show the difference

Herald added subscribers: frasercrmck, luismarques, apazos and 19 others. · View Herald TranscriptJul 9 2021, 2:20 AM
arcbbb edited the summary of this revision. (Show Details)Jul 9 2021, 2:33 AM
arcbbb added reviewers: stoklund, qcolombet, craig.topper, HsiangKai.
arcbbb updated this revision to Diff 357459.Jul 9 2021, 2:46 AM

clang-formatted

Harbormaster completed remote builds in B113159: Diff 357459.Jul 9 2021, 3:20 AM
• post.kadirselcuk added a child revision: D34362: [LNT] Support for different DataSet usage in Polybench for "lnt runtest nt".Jul 10 2021, 5:55 PM
• post.kadirselcuk added a parent revision: D105762: [X86] Teach X86FloatingPoint's handleCall to only erase the FP stack if there is a regmask operand that clobbers the FP stack..Jul 10 2021, 8:06 PM
craig.topper removed a parent revision: D105762: [X86] Teach X86FloatingPoint's handleCall to only erase the FP stack if there is a regmask operand that clobbers the FP stack..Jul 10 2021, 9:47 PM
arcbbb updated this revision to Diff 357842.Jul 11 2021, 11:40 PM

Avoid postfix increment operator.

Harbormaster completed remote builds in B113437: Diff 357842.Jul 12 2021, 12:18 AM
craig.topper removed a child revision: D34362: [LNT] Support for different DataSet usage in Polybench for "lnt runtest nt".Jul 13 2021, 10:13 AM
arcbbb mentioned this in rGbe8159bfa56f: [RISCV][RVV] Precommit a test case for D105684.Jul 17 2021, 9:44 AM
craig.topper added a comment.Jul 20 2021, 5:18 PM

@qcolombet, can you take a look at this?

qcolombet accepted this revision.Jul 21 2021, 9:58 AM

LGTM

This revision is now accepted and ready to land.Jul 21 2021, 9:58 AM
This revision was landed with ongoing or failed builds.Jul 21 2021, 9:11 PM
Closed by commit rG8d86562e5f1f: [RegisterCoalescer] Make resolveConflicts aware of earlyclobber (authored by arcbbb). · Explain Why
This revision was automatically updated to reflect the committed changes.
arcbbb added a commit: rG8d86562e5f1f: [RegisterCoalescer] Make resolveConflicts aware of earlyclobber.

Revision Contents

PathSize
llvm/
lib/
CodeGen/
6 lines
test/
CodeGen/
RISCV/
rvv/
7 lines

Diff 357448

llvm/lib/CodeGen/RegisterCoalescer.cpp

Show First 20 Lines • Show All 3,019 Lines • ▼ Show 20 Lines for (unsigned i = 0, e = LR.getNumValNums(); i != e; ++i) {
assert(!TaintExtent.empty() && "There should be at least one conflict."); assert(!TaintExtent.empty() && "There should be at least one conflict.");
// Now look at the instructions from VNI->def to TaintExtent (inclusive). // Now look at the instructions from VNI->def to TaintExtent (inclusive).
MachineBasicBlock *MBB = Indexes->getMBBFromIndex(VNI->def); MachineBasicBlock *MBB = Indexes->getMBBFromIndex(VNI->def);
MachineBasicBlock::iterator MI = MBB->begin(); MachineBasicBlock::iterator MI = MBB->begin();
if (!VNI->isPHIDef()) { if (!VNI->isPHIDef()) {
MI = Indexes->getInstructionFromIndex(VNI->def); MI = Indexes->getInstructionFromIndex(VNI->def);
if (!VNI->def.isEarlyClobber()) {
// No need to check the instruction defining VNI for reads. // No need to check the instruction defining VNI for reads.
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-          // No need to check the instruction defining VNI for reads.
-          MI++;
+        // No need to check the instruction defining VNI for reads.
+        MI++;
Lint: Pre-merge checks: clang-format: please reformat the code ``` - // No need to check the instruction…
++MI; MI++;
}
} }
assert(!SlotIndex::isSameInstr(VNI->def, TaintExtent.front().first) && assert(!SlotIndex::isSameInstr(VNI->def, TaintExtent.front().first) &&
"Interference ends on VNI->def. Should have been handled earlier"); "Interference ends on VNI->def. Should have been handled earlier");
MachineInstr *LastMI = MachineInstr *LastMI =
Indexes->getInstructionFromIndex(TaintExtent.front().first); Indexes->getInstructionFromIndex(TaintExtent.front().first);
assert(LastMI && "Range must end at a proper instruction"); assert(LastMI && "Range must end at a proper instruction");
unsigned TaintNum = 0; unsigned TaintNum = 0;
while (true) { while (true) {
▲ Show 20 Lines • Show All 1,135 Lines • Show Last 20 Lines

llvm/test/CodeGen/RISCV/rvv/reg-coalescing.mir

# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
# RUN: llc %s -mtriple=riscv64 -mattr=+experimental-v -run-pass=simple-register-coalescing -o - | FileCheck %s # RUN: llc %s -mtriple=riscv64 -mattr=+experimental-v -run-pass=simple-register-coalescing -o - | FileCheck %s
--- ---
# Make sure that SrcReg & DstReg of PseudoVRGATHER are not coalesced # Make sure that SrcReg & DstReg of PseudoVRGATHER are not coalesced
name: test_earlyclobber name: test_earlyclobber
tracksRegLiveness: true tracksRegLiveness: true
body: | body: |
bb.0: bb.0:
liveins: $x10 liveins: $x10
; CHECK-LABEL: name: test_earlyclobber ; CHECK-LABEL: name: test_earlyclobber
; CHECK: liveins: $x10 ; CHECK: liveins: $x10
; CHECK: undef %2.sub_vrm2_0:vrn2m2 = PseudoVLE32_V_M2 $x10, 1, 5 ; CHECK: undef %0.sub_vrm2_0:vrn2m2 = PseudoVLE32_V_M2 $x10, 1, 5
; CHECK: %2.sub_vrm2_1:vrn2m2 = PseudoVLE32_V_M2 $x10, 1, 5 ; CHECK: %0.sub_vrm2_1:vrn2m2 = PseudoVLE32_V_M2 $x10, 1, 5
; CHECK: [[PseudoVLE32_V_M2_:%[0-9]+]]:vrm2 = PseudoVLE32_V_M2 $x10, 1, 5 ; CHECK: [[PseudoVLE32_V_M2_:%[0-9]+]]:vrm2 = PseudoVLE32_V_M2 $x10, 1, 5
; CHECK: early-clobber %2.sub_vrm2_0:vrn2m2 = PseudoVRGATHER_VI_M2 %2.sub_vrm2_0, 0, 1, 5, implicit $vl, implicit $vtype ; CHECK: undef early-clobber %2.sub_vrm2_0:vrn2m2 = PseudoVRGATHER_VI_M2 %0.sub_vrm2_0, 0, 1, 5, implicit $vl, implicit $vtype
; CHECK: %2.sub_vrm2_1:vrn2m2 = COPY %0.sub_vrm2_1
; CHECK: PseudoVSUXSEG2EI32_V_M2_M2 %2, $x10, [[PseudoVLE32_V_M2_]], 1, 5, implicit $vl, implicit $vtype ; CHECK: PseudoVSUXSEG2EI32_V_M2_M2 %2, $x10, [[PseudoVLE32_V_M2_]], 1, 5, implicit $vl, implicit $vtype
undef %0.sub_vrm2_0:vrn2m2 = PseudoVLE32_V_M2 $x10, 1, 5 undef %0.sub_vrm2_0:vrn2m2 = PseudoVLE32_V_M2 $x10, 1, 5
%0.sub_vrm2_1:vrn2m2 = PseudoVLE32_V_M2 $x10, 1, 5 %0.sub_vrm2_1:vrn2m2 = PseudoVLE32_V_M2 $x10, 1, 5
%1:vrm2 = PseudoVLE32_V_M2 $x10, 1, 5 %1:vrm2 = PseudoVLE32_V_M2 $x10, 1, 5
undef early-clobber %2.sub_vrm2_0:vrn2m2 = PseudoVRGATHER_VI_M2 %0.sub_vrm2_0:vrn2m2, 0, 1, 5, implicit $vl, implicit $vtype undef early-clobber %2.sub_vrm2_0:vrn2m2 = PseudoVRGATHER_VI_M2 %0.sub_vrm2_0:vrn2m2, 0, 1, 5, implicit $vl, implicit $vtype
%2.sub_vrm2_1:vrn2m2 = COPY %0.sub_vrm2_1:vrn2m2 %2.sub_vrm2_1:vrn2m2 = COPY %0.sub_vrm2_1:vrn2m2
PseudoVSUXSEG2EI32_V_M2_M2 %2:vrn2m2, $x10, %1:vrm2, 1, 5, implicit $vl, implicit $vtype PseudoVSUXSEG2EI32_V_M2_M2 %2:vrn2m2, $x10, %1:vrm2, 1, 5, implicit $vl, implicit $vtype
... ...