Skip to content

Commit

Permalink
[MIR] Allow overriding isSSA, noPhis, noVRegs in MIR input (llvm#108546)
Browse files Browse the repository at this point in the history
Allow setting the computed properties IsSSA, NoPHIs, NoVRegs for MIR
functions in MIR input. The default value is still the computed value.
If the property is set to false, the computed result is ignored. Conflicting
values (e.g. setting IsSSA where the input MIR is clearly not SSA) lead to
an error.

Closes llvm#37787
  • Loading branch information
gargaroff committed Sep 24, 2024
1 parent 3e3780e commit 8ba334b
Show file tree
Hide file tree
Showing 11 changed files with 170 additions and 18 deletions.
11 changes: 11 additions & 0 deletions llvm/include/llvm/CodeGen/MIRYamlMapping.h
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,11 @@ struct MachineFunction {
bool TracksRegLiveness = false;
bool HasWinCFI = false;

// Computed properties that should be overridable
std::optional<bool> NoPHIs;
std::optional<bool> IsSSA;
std::optional<bool> NoVRegs;

bool CallsEHReturn = false;
bool CallsUnwindInit = false;
bool HasEHCatchret = false;
Expand Down Expand Up @@ -770,6 +775,12 @@ template <> struct MappingTraits<MachineFunction> {
YamlIO.mapOptional("tracksRegLiveness", MF.TracksRegLiveness, false);
YamlIO.mapOptional("hasWinCFI", MF.HasWinCFI, false);

// PHIs must be not be capitalized, since it will clash with the MIR opcode
// leading to false-positive FileCheck hits with CHECK-NOT
YamlIO.mapOptional("noPhis", MF.NoPHIs, std::optional<bool>());
YamlIO.mapOptional("isSSA", MF.IsSSA, std::optional<bool>());
YamlIO.mapOptional("noVRegs", MF.NoVRegs, std::optional<bool>());

YamlIO.mapOptional("callsEHReturn", MF.CallsEHReturn, false);
YamlIO.mapOptional("callsUnwindInit", MF.CallsUnwindInit, false);
YamlIO.mapOptional("hasEHCatchret", MF.HasEHCatchret, false);
Expand Down
52 changes: 41 additions & 11 deletions llvm/lib/CodeGen/MIRParser/MIRParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,8 @@ class MIRParserImpl {
SMDiagnostic diagFromBlockStringDiag(const SMDiagnostic &Error,
SMRange SourceRange);

void computeFunctionProperties(MachineFunction &MF);
bool computeFunctionProperties(MachineFunction &MF,
const yaml::MachineFunction &YamlMF);

void setupDebugValueTracking(MachineFunction &MF,
PerFunctionMIParsingState &PFS, const yaml::MachineFunction &YamlMF);
Expand Down Expand Up @@ -373,7 +374,8 @@ static bool isSSA(const MachineFunction &MF) {
return true;
}

void MIRParserImpl::computeFunctionProperties(MachineFunction &MF) {
bool MIRParserImpl::computeFunctionProperties(
MachineFunction &MF, const yaml::MachineFunction &YamlMF) {
MachineFunctionProperties &Properties = MF.getProperties();

bool HasPHI = false;
Expand All @@ -398,21 +400,48 @@ void MIRParserImpl::computeFunctionProperties(MachineFunction &MF) {
}
}
}
if (!HasPHI)
Properties.set(MachineFunctionProperties::Property::NoPHIs);

// Helper function to sanity-check and set properties that are computed, but
// may be explicitly set from the input MIR
auto ComputedPropertyHelper =
[&Properties](std::optional<bool> ExplicitProp, bool ComputedProp,
MachineFunctionProperties::Property P) -> bool {
// Prefer explicitly given values over the computed properties
if (ExplicitProp.value_or(ComputedProp))
Properties.set(P);
else
Properties.reset(P);

// Check for conflict between the explicit values and the computed ones
return ExplicitProp && *ExplicitProp && !ComputedProp;
};

if (ComputedPropertyHelper(YamlMF.NoPHIs, !HasPHI,
MachineFunctionProperties::Property::NoPHIs)) {
return error(MF.getName() +
" has explicit property NoPhi, but contains at least one PHI");
}

MF.setHasInlineAsm(HasInlineAsm);

if (HasTiedOps && AllTiedOpsRewritten)
Properties.set(MachineFunctionProperties::Property::TiedOpsRewritten);

if (isSSA(MF))
Properties.set(MachineFunctionProperties::Property::IsSSA);
else
Properties.reset(MachineFunctionProperties::Property::IsSSA);
if (ComputedPropertyHelper(YamlMF.IsSSA, isSSA(MF),
MachineFunctionProperties::Property::IsSSA)) {
return error(MF.getName() +
" has explicit property IsSSA, but is not valid SSA");
}

const MachineRegisterInfo &MRI = MF.getRegInfo();
if (MRI.getNumVirtRegs() == 0)
Properties.set(MachineFunctionProperties::Property::NoVRegs);
if (ComputedPropertyHelper(YamlMF.NoVRegs, MRI.getNumVirtRegs() == 0,
MachineFunctionProperties::Property::NoVRegs)) {
return error(
MF.getName() +
" has explicit property NoVRegs, but contains virtual registers");
}

return false;
}

bool MIRParserImpl::initializeCallSiteInfo(
Expand Down Expand Up @@ -595,7 +624,8 @@ MIRParserImpl::initializeMachineFunction(const yaml::MachineFunction &YamlMF,
MachineRegisterInfo &MRI = MF.getRegInfo();
MRI.freezeReservedRegs();

computeFunctionProperties(MF);
if (computeFunctionProperties(MF, YamlMF))
return false;

if (initializeCallSiteInfo(PFS, YamlMF))
return false;
Expand Down
7 changes: 7 additions & 0 deletions llvm/lib/CodeGen/MIRPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,13 @@ void MIRPrinter::print(const MachineFunction &MF) {
YamlMF.TracksDebugUserValues = MF.getProperties().hasProperty(
MachineFunctionProperties::Property::TracksDebugUserValues);

YamlMF.NoPHIs = MF.getProperties().hasProperty(
MachineFunctionProperties::Property::NoPHIs);
YamlMF.IsSSA = MF.getProperties().hasProperty(
MachineFunctionProperties::Property::IsSSA);
YamlMF.NoVRegs = MF.getProperties().hasProperty(
MachineFunctionProperties::Property::NoVRegs);

convert(YamlMF, MF.getRegInfo(), MF.getSubtarget().getRegisterInfo());
MachineModuleSlotTracker MST(MMI, &MF);
MST.incorporateFunction(MF.getFunction());
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/CodeGen/AArch64/mlicm-stack-write-check.mir
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
---
name: test
tracksRegLiveness: true
isSSA: false
registers:
- { id: 0, class: gpr64 }
stack:
Expand Down Expand Up @@ -30,11 +31,11 @@ body: |
bb.2:
liveins: $x0
%0 = COPY $x0
%0 = COPY $x0 ; Force isSSA = false.
...
---
name: test2
tracksRegLiveness: true
isSSA: false
registers:
- { id: 0, class: gpr64 }
stack:
Expand Down Expand Up @@ -62,5 +63,4 @@ body: |
bb.2:
liveins: $x0
%0 = COPY $x0
%0 = COPY $x0 ; Force isSSA = false.
...
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/Hexagon/expand-condsets-impuse2.mir
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@

name: f0
tracksRegLiveness: true
isSSA: false
body: |
bb.0:
successors: %bb.1
liveins: $r0, $r1
%0:intregs = COPY $r0
%0:intregs = COPY $r0 ; defeat IsSSA detection
%1:intregs = COPY $r1
%2:intregs = COPY $r0
%3:intregs = M2_mpyi %2, %1
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/Hexagon/expand-condsets-phys-reg.mir
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@

name: fred
tracksRegLiveness: true
isSSA: false
body: |
bb.0:
successors: %bb.1, %bb.2
liveins: $r0
%0:intregs = A2_tfrsi 0 ;; Multiple defs to ensure IsSSA = false
%0:intregs = L2_loadri_io $r0, 0
%1:predregs = C2_cmpgti %0, 10
%2:intregs = C2_mux %1, $r31, %0
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/Hexagon/expand-condsets-rm-reg.mir
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

name: fred
tracksRegLiveness: true
isSSA: false
registers:
- { id: 0, class: intregs }
- { id: 1, class: intregs }
Expand All @@ -35,7 +36,6 @@ body: |
bb.0:
liveins: $r0, $r1, $p0
%0 = COPY $r0
%0 = COPY $r0 ; Force isSSA = false.
%1 = COPY $r1
%2 = COPY $p0
; Check that %3 was coalesced into %4.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# RUN: not llc -run-pass none -o /dev/null %s 2>&1 | FileCheck %s

# Test that computed properties are not conflicting with explicitly set
# properties

---
# CHECK: error: {{.*}}: TestNoPhisOverrideConflict has explicit property NoPhi, but contains at least one PHI
name: TestNoPhisOverrideConflict
noPhis: true
tracksRegLiveness: true
body: |
bb.0:
%0:_(s32) = G_IMPLICIT_DEF
bb.1:
%1:_(s32) = PHI %0, %bb.0, %1, %bb.1
G_BR %bb.1
...
---
# CHECK: error: {{.*}}: TestIsSSAOverrideConflict has explicit property IsSSA, but is not valid SSA
name: TestIsSSAOverrideConflict
isSSA: true
body: |
bb.0:
%0:_(s32) = G_IMPLICIT_DEF
%0:_(s32) = G_IMPLICIT_DEF
...
---
# CHECK: error: {{.*}}: TestNoVRegsOverrideConflict has explicit property NoVRegs, but contains virtual registers
name: TestNoVRegsOverrideConflict
noVRegs: true
body: |
bb.0:
%0:_(s32) = G_IMPLICIT_DEF
...
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# RUN: llc -run-pass none -o - %s | FileCheck %s

# Test that we can disable certain properties that are normally computed

---
# CHECK-LABEL: name: TestNoPhis
# CHECK: noPhis: true
# CHECK: ...
name: TestNoPhis
...
---
# CHECK-LABEL: name: TestNoPhisOverride
# CHECK: noPhis: false
# CHECK: ...
name: TestNoPhisOverride
noPhis: false
...
---
# CHECK-LABEL: name: TestNoPhisOverrideTrue
# CHECK: noPhis: true
# CHECK: ...
name: TestNoPhisOverrideTrue
noPhis: true
...
---
# CHECK-LABEL: name: TestIsSSA
# CHECK: isSSA: true
# CHECK: ...
name: TestIsSSA
...
---
# CHECK-LABEL: name: TestIsSSAOverride
# CHECK: isSSA: false
# CHECK: ...
name: TestIsSSAOverride
isSSA: false
...
---
# CHECK-LABEL: name: TestIsSSAOverrideTrue
# CHECK: isSSA: true
# CHECK: ...
name: TestIsSSAOverrideTrue
isSSA: true
...
---
# CHECK-LABEL: name: TestNoVRegs
# CHECK: noVRegs: true
# CHECK: ...
name: TestNoVRegs
...
---
# CHECK-LABEL: name: TestNoVRegsOverride
# CHECK: noVRegs: false
# CHECK: ...
name: TestNoVRegsOverride
noVRegs: false
...
---
# CHECK-LABEL: name: TestNoVRegsOverrideTrue
# CHECK: noVRegs: true
# CHECK: ...
name: TestNoVRegsOverrideTrue
noVRegs: true
...
3 changes: 1 addition & 2 deletions llvm/test/CodeGen/X86/sjlj-shadow-stack-liveness.mir
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ name: bar
# CHECK-LABEL: name: bar
alignment: 16
tracksRegLiveness: true
noPhis: false
body: |
bb.0:
%0:gr64 = IMPLICIT_DEF
Expand All @@ -29,8 +30,6 @@ body: |
; CHECK-NOT: MOV64rm killed %0
; CHECK-NEXT: MOV64rm killed %0
; FIXME: Dummy PHI to set the property NoPHIs to false. PR38439.
bb.2:
%1:gr64 = PHI undef %1, %bb.2, undef %1, %bb.2
JMP_1 %bb.2
...
6 changes: 6 additions & 0 deletions llvm/test/tools/llvm-reduce/mir/preserve-func-info.mir
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
# RESULT-NEXT: failedISel: true
# RESULT-NEXT: tracksRegLiveness: true
# RESULT-NEXT: hasWinCFI: true
# RESULT-NEXT: noPhis: false
# RESULT-NEXT: isSSA: false
# RESULT-NEXT: noVRegs: false
# RESULT-NEXT: callsEHReturn: true
# RESULT-NEXT: callsUnwindInit: true
# RESULT-NEXT: hasEHCatchret: true
Expand Down Expand Up @@ -41,6 +44,9 @@ selected: true
failedISel: true
tracksRegLiveness: true
hasWinCFI: true
noPhis: false
isSSA: false
noVRegs: false
failsVerification: true
tracksDebugUserValues: true
callsEHReturn: true
Expand Down

0 comments on commit 8ba334b

Please sign in to comment.