Skip to content

[LiveIntervals] Ignore artificial regs when adding kill flags #116963

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Dec 4, 2024
Merged

[LiveIntervals] Ignore artificial regs when adding kill flags #116963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4c61f6f
Precommit tests
sdesmalen-arm Nov 20, 2024
3b40757
[LiveIntervals] Ignore artificial regs when adding kill flags
sdesmalen-arm Nov 20, 2024
473ce09
Precompute subreg info with TableGen
sdesmalen-arm Dec 3, 2024
64bf6e4
Use MCRegUnitMaskIterator instead
sdesmalen-arm Dec 4, 2024
a84983c
NFC: Move check for artificial regunit to MachineRegisterInfo
sdesmalen-arm Dec 4, 2024
3ac2c4a
Move isArtificialRegUnit to MCRegisterInfo
sdesmalen-arm Dec 4, 2024
dbcd858
Address nit
sdesmalen-arm Dec 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions llvm/include/llvm/MC/MCRegisterInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,11 @@ class MCRegisterInfo {
/// be modelled, such as the top 16-bits of a 32-bit GPR.
bool isArtificial(MCRegister RegNo) const { return get(RegNo).IsArtificial; }

/// Returns true when the given register unit is considered artificial.
/// Register units are considered artificial when at least one of the
/// root registers is artificial.
bool isArtificialRegUnit(MCRegUnit Unit) const;

/// Return the number of registers this target has (useful for
/// sizing arrays holding per register information)
unsigned getNumRegs() const {
Expand Down
13 changes: 11 additions & 2 deletions llvm/lib/CodeGen/LiveIntervals.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,12 @@ void LiveIntervals::addKillFlags(const VirtRegMap *VRM) {
// Find the regunit intervals for the assigned register. They may overlap
// the virtual register live range, cancelling any kills.
RU.clear();
for (MCRegUnit Unit : TRI->regunits(PhysReg)) {
LaneBitmask ArtificialLanes;
for (MCRegUnitMaskIterator UI(PhysReg, TRI); UI.isValid(); ++UI) {
auto [Unit, Bitmask] = *UI;
// Record lane mask for all artificial RegUnits for this physreg.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tablegen calculates the "aritificial" property for regunits as well as registers, but I guess we don't have direct access to that here?

llvm-project/llvm/utils/TableGen/Common/CodeGenRegisters.h

Line 550 in 041e5c9

// A register unit is artificial if at least one of its roots is

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not directly, unless I generate a table for it. Iterating through the roots shouldn't be much of a problem in practice, since there can never be more than two roots. (It's actually not clear to me why there would ever be more than one root? What's the use-case for this?)

In either case, I'll move this to a special isArtificial(MCRegUnit) method to query this info, to avoid having to write this loop in various places in the future. If you think it's worth generating this in a table though, I'm happy to do so!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually not clear to me why there would ever be more than one root? What's the use-case for this?

I think the second root is only ever an aritificial register used to represent ad hoc aliasing (the Aliases = [...] field in .td files), but I could be misremembering the details.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at ARMGenRegisterInfo.td, I think that's indeed right, thanks for explaining!

if (TRI->isArtificialRegUnit(Unit))
ArtificialLanes |= Bitmask;
const LiveRange &RURange = getRegUnit(Unit);
if (RURange.empty())
continue;
Expand Down Expand Up @@ -782,7 +787,11 @@ void LiveIntervals::addKillFlags(const VirtRegMap *VRM) {
LaneBitmask DefinedLanesMask;
if (LI.hasSubRanges()) {
// Compute a mask of lanes that are defined.
DefinedLanesMask = LaneBitmask::getNone();
// Artificial regunits are not independently allocatable so the
// register allocator cannot have used them to represent any other
// values. That's why we mark them as 'defined' here, as this
// otherwise prevents kill flags from being added.
DefinedLanesMask = ArtificialLanes;
for (const LiveInterval::SubRange &SR : LI.subranges())
for (const LiveRange::Segment &Segment : SR.segments) {
if (Segment.start >= RI->end)
Expand Down
7 changes: 7 additions & 0 deletions llvm/lib/MC/MCRegisterInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,3 +220,10 @@ bool MCRegisterInfo::regsOverlap(MCRegister RegA, MCRegister RegB) const {
} while (*IA < *IB ? ++IA != EA : ++IB != EB);
return false;
}

bool MCRegisterInfo::isArtificialRegUnit(unsigned Unit) const {
for (MCRegUnitRootIterator Root(Unit, this); Root.isValid(); ++Root)
if (isArtificial(*Root))
return true;
return false;
}
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/AArch64/arm64-addrmode.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc -mtriple=arm64-eabi < %s | FileCheck %s
; RUN: llc -aarch64-enable-subreg-liveness-tracking -mtriple=arm64-eabi < %s | FileCheck %s
; rdar://10232252

@object = external hidden global i64, section "__DATA, __objc_ivar", align 8
Expand Down
6 changes: 3 additions & 3 deletions llvm/test/CodeGen/AArch64/nested-iv-regalloc.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
# RUN: llc -mtriple aarch64 --run-pass=greedy,virtregrewriter -verify-machineinstrs %s -o - | FileCheck %s
# RUN: llc -mtriple aarch64 -aarch64-enable-subreg-liveness-tracking --run-pass=greedy,virtregrewriter -verify-machineinstrs %s -o - | FileCheck %s

# We should ideally not spill around any of the SUBSWri in the loop exit blocks (if.end and if.end27).

Expand Down Expand Up @@ -219,7 +219,7 @@ body: |
; CHECK-NEXT: liveins: $w10, $w11, $x2, $x8
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: STRXui renamable $x8, %stack.1, 0 :: (store (s64) into %stack.1)
; CHECK-NEXT: renamable $w9 = MOVi32imm 36, implicit-def $x9
; CHECK-NEXT: renamable $w9 = MOVi32imm 36
; CHECK-NEXT: renamable $x8 = MADDXrrr killed renamable $x8, killed renamable $x9, $xzr
; CHECK-NEXT: renamable $x9 = MOVaddr target-flags(aarch64-page) @g, target-flags(aarch64-pageoff, aarch64-nc) @g
; CHECK-NEXT: renamable $w8 = LDRWroX killed renamable $x9, killed renamable $x8, 0, 0 :: (load (s32) from %ir.arrayidx9)
Expand All @@ -244,7 +244,7 @@ body: |
; CHECK-NEXT: successors: %bb.5(0x50000000), %bb.8(0x30000000)
; CHECK-NEXT: liveins: $w10, $w11, $x2, $x12
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: renamable $w8 = MOVi32imm 36, implicit-def $x8
; CHECK-NEXT: renamable $w8 = MOVi32imm 36
; CHECK-NEXT: renamable $x8 = MADDXrrr renamable $x12, killed renamable $x8, $xzr
; CHECK-NEXT: renamable $x9 = MOVaddr target-flags(aarch64-page) @g, target-flags(aarch64-pageoff, aarch64-nc) @g
; CHECK-NEXT: renamable $w8 = LDRWroX killed renamable $x9, killed renamable $x8, 0, 0 :: (load (s32) from %ir.arrayidx14)
Expand Down
12 changes: 6 additions & 6 deletions llvm/test/CodeGen/AArch64/preserve_nonecc_varargs_darwin.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
; RUN: llc -mtriple=aarch64-apple-darwin < %s | FileCheck %s
; RUN: llc -mtriple=aarch64-apple-darwin -aarch64-enable-subreg-liveness-tracking < %s | FileCheck %s

define preserve_nonecc i32 @callee(i32 %a1, i32 %a2, i32 %a3, i32 %a4, i32 %a5, ...) nounwind noinline ssp {
; CHECK-LABEL: callee:
Expand Down Expand Up @@ -27,12 +27,11 @@ define i32 @caller() nounwind ssp {
; CHECK-NEXT: sub sp, sp, #208
; CHECK-NEXT: mov w8, #10 ; =0xa
; CHECK-NEXT: mov w9, #9 ; =0x9
; CHECK-NEXT: mov w0, #1 ; =0x1
; CHECK-NEXT: mov w10, #8 ; =0x8
; CHECK-NEXT: stp x9, x8, [sp, #24]
; CHECK-NEXT: mov w8, #8 ; =0x8
; CHECK-NEXT: mov w9, #6 ; =0x6
; CHECK-NEXT: str x8, [sp, #16]
; CHECK-NEXT: mov w8, #7 ; =0x7
; CHECK-NEXT: mov w9, #6 ; =0x6
; CHECK-NEXT: mov w0, #1 ; =0x1
; CHECK-NEXT: mov w1, #2 ; =0x2
; CHECK-NEXT: mov w2, #3 ; =0x3
; CHECK-NEXT: mov w3, #4 ; =0x4
Expand All @@ -47,7 +46,8 @@ define i32 @caller() nounwind ssp {
; CHECK-NEXT: stp x22, x21, [sp, #160] ; 16-byte Folded Spill
; CHECK-NEXT: stp x20, x19, [sp, #176] ; 16-byte Folded Spill
; CHECK-NEXT: stp x29, x30, [sp, #192] ; 16-byte Folded Spill
; CHECK-NEXT: stp x9, x8, [sp]
; CHECK-NEXT: stp x8, x10, [sp, #8]
; CHECK-NEXT: str x9, [sp]
; CHECK-NEXT: bl _callee
; CHECK-NEXT: ldp x29, x30, [sp, #192] ; 16-byte Folded Reload
; CHECK-NEXT: ldp x20, x19, [sp, #176] ; 16-byte Folded Reload
Expand Down
Loading