[llvm] [MIR] Allow overriding isSSA, noPhis, noVRegs in MIR input (PR #108546)
Dominik Montada via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 24 05:04:42 PDT 2024
https://github.com/gargaroff updated https://github.com/llvm/llvm-project/pull/108546
>From 3ef9706838daf14bf36d03e22e508d417b9c77aa Mon Sep 17 00:00:00 2001
From: Dominik Montada <dominik.montada at arm.com>
Date: Fri, 13 Sep 2024 13:45:25 +0200
Subject: [PATCH 1/6] [MIR] Allow overriding isSSA, noPhis, noVRegs in MIR
input
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. This
allows for tests where a pass is for example inserting PHI nodes into a
function that didn't have any previously.
Closes #37787
---
llvm/include/llvm/CodeGen/MIRYamlMapping.h | 11 +++++
llvm/lib/CodeGen/MIRParser/MIRParser.cpp | 19 ++++++---
llvm/lib/CodeGen/MIRPrinter.cpp | 7 ++++
.../AArch64/mlicm-stack-write-check.mir | 4 +-
.../AMDGPU/early-tailduplicator-nophis.mir | 4 +-
.../Hexagon/expand-condsets-impuse2.mir | 2 +-
.../Hexagon/expand-condsets-phys-reg.mir | 2 +-
.../Hexagon/expand-condsets-rm-reg.mir | 2 +-
...unction-optionally-computed-properties.mir | 42 +++++++++++++++++++
.../X86/sjlj-shadow-stack-liveness.mir | 3 +-
.../llvm-reduce/mir/preserve-func-info.mir | 6 +++
11 files changed, 88 insertions(+), 14 deletions(-)
create mode 100644 llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties.mir
diff --git a/llvm/include/llvm/CodeGen/MIRYamlMapping.h b/llvm/include/llvm/CodeGen/MIRYamlMapping.h
index 304db57eca4994..e90c22284dcceb 100644
--- a/llvm/include/llvm/CodeGen/MIRYamlMapping.h
+++ b/llvm/include/llvm/CodeGen/MIRYamlMapping.h
@@ -730,6 +730,11 @@ struct MachineFunction {
bool TracksRegLiveness = false;
bool HasWinCFI = false;
+ // Computed properties that should be overridable
+ bool NoPHIs = false;
+ bool IsSSA = false;
+ bool NoVRegs = false;
+
bool CallsEHReturn = false;
bool CallsUnwindInit = false;
bool HasEHCatchret = false;
@@ -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, true);
+ YamlIO.mapOptional("isSSA", MF.IsSSA, true);
+ YamlIO.mapOptional("noVRegs", MF.NoVRegs, true);
+
YamlIO.mapOptional("callsEHReturn", MF.CallsEHReturn, false);
YamlIO.mapOptional("callsUnwindInit", MF.CallsUnwindInit, false);
YamlIO.mapOptional("hasEHCatchret", MF.HasEHCatchret, false);
diff --git a/llvm/lib/CodeGen/MIRParser/MIRParser.cpp b/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
index d506cd1879648f..a3af435bd2bf07 100644
--- a/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
+++ b/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
@@ -178,7 +178,8 @@ class MIRParserImpl {
SMDiagnostic diagFromBlockStringDiag(const SMDiagnostic &Error,
SMRange SourceRange);
- void computeFunctionProperties(MachineFunction &MF);
+ void computeFunctionProperties(MachineFunction &MF,
+ const yaml::MachineFunction &YamlMF);
void setupDebugValueTracking(MachineFunction &MF,
PerFunctionMIParsingState &PFS, const yaml::MachineFunction &YamlMF);
@@ -373,7 +374,8 @@ static bool isSSA(const MachineFunction &MF) {
return true;
}
-void MIRParserImpl::computeFunctionProperties(MachineFunction &MF) {
+void MIRParserImpl::computeFunctionProperties(
+ MachineFunction &MF, const yaml::MachineFunction &YamlMF) {
MachineFunctionProperties &Properties = MF.getProperties();
bool HasPHI = false;
@@ -398,20 +400,25 @@ void MIRParserImpl::computeFunctionProperties(MachineFunction &MF) {
}
}
}
- if (!HasPHI)
+
+ // Don't overwrite NoPHIs if the input MIR explicitly set it to false
+ if (YamlMF.NoPHIs && !HasPHI)
Properties.set(MachineFunctionProperties::Property::NoPHIs);
+
MF.setHasInlineAsm(HasInlineAsm);
if (HasTiedOps && AllTiedOpsRewritten)
Properties.set(MachineFunctionProperties::Property::TiedOpsRewritten);
- if (isSSA(MF))
+ // Don't overwrite IsSSA if the input MIR explicitly set it to false
+ if (YamlMF.IsSSA && isSSA(MF))
Properties.set(MachineFunctionProperties::Property::IsSSA);
else
Properties.reset(MachineFunctionProperties::Property::IsSSA);
+ // Don't overwrite NoVRegs if the input MIR explicitly set it to false
const MachineRegisterInfo &MRI = MF.getRegInfo();
- if (MRI.getNumVirtRegs() == 0)
+ if (YamlMF.NoVRegs && MRI.getNumVirtRegs() == 0)
Properties.set(MachineFunctionProperties::Property::NoVRegs);
}
@@ -595,7 +602,7 @@ MIRParserImpl::initializeMachineFunction(const yaml::MachineFunction &YamlMF,
MachineRegisterInfo &MRI = MF.getRegInfo();
MRI.freezeReservedRegs();
- computeFunctionProperties(MF);
+ computeFunctionProperties(MF, YamlMF);
if (initializeCallSiteInfo(PFS, YamlMF))
return false;
diff --git a/llvm/lib/CodeGen/MIRPrinter.cpp b/llvm/lib/CodeGen/MIRPrinter.cpp
index 7de68b12045f14..cf6122bce22364 100644
--- a/llvm/lib/CodeGen/MIRPrinter.cpp
+++ b/llvm/lib/CodeGen/MIRPrinter.cpp
@@ -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());
diff --git a/llvm/test/CodeGen/AArch64/mlicm-stack-write-check.mir b/llvm/test/CodeGen/AArch64/mlicm-stack-write-check.mir
index 51bc77d405b94b..406025c4fde302 100644
--- a/llvm/test/CodeGen/AArch64/mlicm-stack-write-check.mir
+++ b/llvm/test/CodeGen/AArch64/mlicm-stack-write-check.mir
@@ -3,6 +3,7 @@
---
name: test
tracksRegLiveness: true
+isSSA: false
registers:
- { id: 0, class: gpr64 }
stack:
@@ -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:
@@ -62,5 +63,4 @@ body: |
bb.2:
liveins: $x0
%0 = COPY $x0
- %0 = COPY $x0 ; Force isSSA = false.
...
diff --git a/llvm/test/CodeGen/AMDGPU/early-tailduplicator-nophis.mir b/llvm/test/CodeGen/AMDGPU/early-tailduplicator-nophis.mir
index 2cb84c7ef4637d..9fcbdac0ff9d1f 100644
--- a/llvm/test/CodeGen/AMDGPU/early-tailduplicator-nophis.mir
+++ b/llvm/test/CodeGen/AMDGPU/early-tailduplicator-nophis.mir
@@ -2,11 +2,13 @@
# RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 -run-pass=early-tailduplication -verify-machineinstrs -o - %s | FileCheck %s
# There are no phis in this testcase. Early tail duplication introduces them,
- # so the NoPHIs property needs to be cleared to avoid verifier errors
+ # so the NoPHIs property needs to be set explicitly to false to avoid verifier
+ # errors
---
name: tail_duplicate_nophis
tracksRegLiveness: true
+noPhis: false
body: |
; CHECK-LABEL: name: tail_duplicate_nophis
; CHECK: bb.0:
diff --git a/llvm/test/CodeGen/Hexagon/expand-condsets-impuse2.mir b/llvm/test/CodeGen/Hexagon/expand-condsets-impuse2.mir
index ae3f4ba78cd1ff..ebb361ab433cb7 100644
--- a/llvm/test/CodeGen/Hexagon/expand-condsets-impuse2.mir
+++ b/llvm/test/CodeGen/Hexagon/expand-condsets-impuse2.mir
@@ -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
diff --git a/llvm/test/CodeGen/Hexagon/expand-condsets-phys-reg.mir b/llvm/test/CodeGen/Hexagon/expand-condsets-phys-reg.mir
index e62cd1cc73609b..d252ec5fee4019 100644
--- a/llvm/test/CodeGen/Hexagon/expand-condsets-phys-reg.mir
+++ b/llvm/test/CodeGen/Hexagon/expand-condsets-phys-reg.mir
@@ -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
diff --git a/llvm/test/CodeGen/Hexagon/expand-condsets-rm-reg.mir b/llvm/test/CodeGen/Hexagon/expand-condsets-rm-reg.mir
index 6d7b6cd72a3099..463aa9a8e7f9b1 100644
--- a/llvm/test/CodeGen/Hexagon/expand-condsets-rm-reg.mir
+++ b/llvm/test/CodeGen/Hexagon/expand-condsets-rm-reg.mir
@@ -20,6 +20,7 @@
name: fred
tracksRegLiveness: true
+isSSA: false
registers:
- { id: 0, class: intregs }
- { id: 1, class: intregs }
@@ -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.
diff --git a/llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties.mir b/llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties.mir
new file mode 100644
index 00000000000000..c833e380480021
--- /dev/null
+++ b/llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties.mir
@@ -0,0 +1,42 @@
+# 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: TestIsSSA
+# CHECK: isSSA: true
+# CHECK: ...
+name: TestIsSSA
+...
+---
+# CHECK-LABEL: name: TestIsSSAOverride
+# CHECK: isSSA: false
+# CHECK: ...
+name: TestIsSSAOverride
+isSSA: false
+...
+---
+# CHECK-LABEL: name: TestNoVRegs
+# CHECK: noVRegs: true
+# CHECK: ...
+name: TestNoVRegs
+...
+---
+# CHECK-LABEL: name: TestNoVRegsOverride
+# CHECK: noVRegs: false
+# CHECK: ...
+name: TestNoVRegsOverride
+noVRegs: false
+...
diff --git a/llvm/test/CodeGen/X86/sjlj-shadow-stack-liveness.mir b/llvm/test/CodeGen/X86/sjlj-shadow-stack-liveness.mir
index 3def36f9d8ba91..83bc8ec510f646 100644
--- a/llvm/test/CodeGen/X86/sjlj-shadow-stack-liveness.mir
+++ b/llvm/test/CodeGen/X86/sjlj-shadow-stack-liveness.mir
@@ -14,6 +14,7 @@ name: bar
# CHECK-LABEL: name: bar
alignment: 16
tracksRegLiveness: true
+noPhis: false
body: |
bb.0:
%0:gr64 = IMPLICIT_DEF
@@ -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
...
diff --git a/llvm/test/tools/llvm-reduce/mir/preserve-func-info.mir b/llvm/test/tools/llvm-reduce/mir/preserve-func-info.mir
index 5f11cea89d7e7b..f735dfd5cbbf01 100644
--- a/llvm/test/tools/llvm-reduce/mir/preserve-func-info.mir
+++ b/llvm/test/tools/llvm-reduce/mir/preserve-func-info.mir
@@ -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
@@ -41,6 +44,9 @@ selected: true
failedISel: true
tracksRegLiveness: true
hasWinCFI: true
+noPhis: false
+isSSA: false
+noVRegs: false
failsVerification: true
tracksDebugUserValues: true
callsEHReturn: true
>From aa80b71570882fbaa5dc992d9781acdfe4999cff Mon Sep 17 00:00:00 2001
From: Dominik Montada <dominik.montada at arm.com>
Date: Mon, 16 Sep 2024 09:13:01 +0200
Subject: [PATCH 2/6] Add tests for explicit true values and conflicting values
---
...unction-optionally-computed-properties.mir | 63 ++++++++++++++++++-
1 file changed, 62 insertions(+), 1 deletion(-)
diff --git a/llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties.mir b/llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties.mir
index c833e380480021..0cbdd8e1579430 100644
--- a/llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties.mir
+++ b/llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties.mir
@@ -1,5 +1,9 @@
# RUN: llc -run-pass none -o - %s | FileCheck %s
-# Test that we can disable certain properties that are normally computed
+# Test that we can disable certain properties that are normally computed. This
+# override is a soft-override (only allows overriding when it relaxes the
+# property). Therefore, a conflicting override (e.g. setting noPhis to true in
+# the presence of PHI nodes) should not result in an error, but instead should
+# use the computed property instead (noPhis = false in the mentioned example).
---
# CHECK-LABEL: name: TestNoPhis
@@ -15,6 +19,28 @@ name: TestNoPhisOverride
noPhis: false
...
---
+# CHECK-LABEL: name: TestNoPhisOverrideTrue
+# CHECK: noPhis: true
+# CHECK: ...
+name: TestNoPhisOverrideTrue
+noPhis: true
+...
+---
+# CHECK-LABEL: name: TestNoPhisOverrideConflict
+# CHECK: noPhis: false
+# CHECK: ...
+name: TestNoPhisOverrideConflict
+noPhis: true
+tracksRegLiveness: true
+body: |
+ bb.0:
+ %0:_(s32) = IMPLICIT_DEF
+
+ bb.1:
+ %1:_(s32) = PHI %0, %bb.0, %0, %bb.1
+ G_BR %bb.1
+...
+---
# CHECK-LABEL: name: TestIsSSA
# CHECK: isSSA: true
# CHECK: ...
@@ -28,6 +54,24 @@ name: TestIsSSAOverride
isSSA: false
...
---
+# CHECK-LABEL: name: TestIsSSAOverrideTrue
+# CHECK: isSSA: true
+# CHECK: ...
+name: TestIsSSAOverrideTrue
+isSSA: true
+...
+---
+# CHECK-LABEL: name: TestIsSSAOverrideConflict
+# CHECK: isSSA: false
+# CHECK: ...
+name: TestIsSSAOverrideConflict
+isSSA: true
+body: |
+ bb.0:
+ %0:_(s32) = IMPLICIT_DEF
+ %0:_(s32) = IMPLICIT_DEF
+...
+---
# CHECK-LABEL: name: TestNoVRegs
# CHECK: noVRegs: true
# CHECK: ...
@@ -40,3 +84,20 @@ name: TestNoVRegs
name: TestNoVRegsOverride
noVRegs: false
...
+---
+# CHECK-LABEL: name: TestNoVRegsOverrideTrue
+# CHECK: noVRegs: true
+# CHECK: ...
+name: TestNoVRegsOverrideTrue
+noVRegs: true
+...
+---
+# CHECK-LABEL: name: TestNoVRegsOverrideConflict
+# CHECK: noVRegs: false
+# CHECK: ...
+name: TestNoVRegsOverrideConflict
+noVRegs: true
+body: |
+ bb.0:
+ %0:_(s32) = IMPLICIT_DEF
+...
>From d9c9e48bff64e303cdd7b2ffe0f33f2887e09ae4 Mon Sep 17 00:00:00 2001
From: Dominik Montada <dominik.montada at arm.com>
Date: Thu, 19 Sep 2024 13:31:06 +0200
Subject: [PATCH 3/6] Revert redundant changes to tests
---
llvm/test/CodeGen/AMDGPU/early-tailduplicator-nophis.mir | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/llvm/test/CodeGen/AMDGPU/early-tailduplicator-nophis.mir b/llvm/test/CodeGen/AMDGPU/early-tailduplicator-nophis.mir
index 9fcbdac0ff9d1f..2cb84c7ef4637d 100644
--- a/llvm/test/CodeGen/AMDGPU/early-tailduplicator-nophis.mir
+++ b/llvm/test/CodeGen/AMDGPU/early-tailduplicator-nophis.mir
@@ -2,13 +2,11 @@
# RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 -run-pass=early-tailduplication -verify-machineinstrs -o - %s | FileCheck %s
# There are no phis in this testcase. Early tail duplication introduces them,
- # so the NoPHIs property needs to be set explicitly to false to avoid verifier
- # errors
+ # so the NoPHIs property needs to be cleared to avoid verifier errors
---
name: tail_duplicate_nophis
tracksRegLiveness: true
-noPhis: false
body: |
; CHECK-LABEL: name: tail_duplicate_nophis
; CHECK: bb.0:
>From 0bbb60d0a09b5f0153d5b5a66f22d34dc384fe15 Mon Sep 17 00:00:00 2001
From: Dominik Montada <dominik.montada at arm.com>
Date: Mon, 23 Sep 2024 12:27:54 +0200
Subject: [PATCH 4/6] Make conflicting properties a hard error instead of
preferring the less strict one
---
llvm/include/llvm/CodeGen/MIRYamlMapping.h | 12 ++--
llvm/lib/CodeGen/MIRParser/MIRParser.cpp | 64 +++++++++++++++----
...ptionally-computed-properties-conflict.mir | 38 +++++++++++
...unction-optionally-computed-properties.mir | 43 +------------
4 files changed, 96 insertions(+), 61 deletions(-)
create mode 100644 llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties-conflict.mir
diff --git a/llvm/include/llvm/CodeGen/MIRYamlMapping.h b/llvm/include/llvm/CodeGen/MIRYamlMapping.h
index e90c22284dcceb..ab8dc442e04b7b 100644
--- a/llvm/include/llvm/CodeGen/MIRYamlMapping.h
+++ b/llvm/include/llvm/CodeGen/MIRYamlMapping.h
@@ -731,9 +731,9 @@ struct MachineFunction {
bool HasWinCFI = false;
// Computed properties that should be overridable
- bool NoPHIs = false;
- bool IsSSA = false;
- bool NoVRegs = false;
+ std::optional<bool> NoPHIs;
+ std::optional<bool> IsSSA;
+ std::optional<bool> NoVRegs;
bool CallsEHReturn = false;
bool CallsUnwindInit = false;
@@ -777,9 +777,9 @@ template <> struct MappingTraits<MachineFunction> {
// 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, true);
- YamlIO.mapOptional("isSSA", MF.IsSSA, true);
- YamlIO.mapOptional("noVRegs", MF.NoVRegs, true);
+ 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);
diff --git a/llvm/lib/CodeGen/MIRParser/MIRParser.cpp b/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
index a3af435bd2bf07..8c0288d789216c 100644
--- a/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
+++ b/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
@@ -178,7 +178,7 @@ 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,
@@ -374,7 +374,7 @@ static bool isSSA(const MachineFunction &MF) {
return true;
}
-void MIRParserImpl::computeFunctionProperties(
+bool MIRParserImpl::computeFunctionProperties(
MachineFunction &MF, const yaml::MachineFunction &YamlMF) {
MachineFunctionProperties &Properties = MF.getProperties();
@@ -401,25 +401,60 @@ void MIRParserImpl::computeFunctionProperties(
}
}
- // Don't overwrite NoPHIs if the input MIR explicitly set it to false
- if (YamlMF.NoPHIs && !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 whatever is set explicitly by the input MIR
+ if (ExplicitProp.has_value()) {
+ if (*ExplicitProp) {
+ // Check for conflicts with the computed value
+ if (!ComputedProp)
+ return true;
+
+ Properties.set(P);
+ } else
+ Properties.reset(P);
+
+ return false;
+ }
+
+ // No explicit value given, so use computed value
+ if (ComputedProp)
+ Properties.set(P);
+ else
+ Properties.reset(P);
+
+ return false;
+ };
+
+ 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);
- // Don't overwrite IsSSA if the input MIR explicitly set it to false
- if (YamlMF.IsSSA && 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");
+ }
- // Don't overwrite NoVRegs if the input MIR explicitly set it to false
const MachineRegisterInfo &MRI = MF.getRegInfo();
- if (YamlMF.NoVRegs && 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(
@@ -602,7 +637,8 @@ MIRParserImpl::initializeMachineFunction(const yaml::MachineFunction &YamlMF,
MachineRegisterInfo &MRI = MF.getRegInfo();
MRI.freezeReservedRegs();
- computeFunctionProperties(MF, YamlMF);
+ if (computeFunctionProperties(MF, YamlMF))
+ return false;
if (initializeCallSiteInfo(PFS, YamlMF))
return false;
diff --git a/llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties-conflict.mir b/llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties-conflict.mir
new file mode 100644
index 00000000000000..153a017d970e9a
--- /dev/null
+++ b/llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties-conflict.mir
@@ -0,0 +1,38 @@
+# 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-LABEL: TestNoPhisOverrideConflict
+# CHECK-SAME: has explicit property NoPhi, but contains at least one PHI
+name: TestNoPhisOverrideConflict
+noPhis: true
+tracksRegLiveness: true
+body: |
+ bb.0:
+ %0:_(s32) = IMPLICIT_DEF
+
+ bb.1:
+ %1:_(s32) = PHI %0, %bb.0, %1, %bb.1
+ G_BR %bb.1
+...
+---
+# CHECK-LABEL: TestIsSSAOverrideConflict
+# CHECK-SAME: has explicit property IsSSA, but is not valid SSA
+name: TestIsSSAOverrideConflict
+isSSA: true
+body: |
+ bb.0:
+ %0:_(s32) = IMPLICIT_DEF
+ %0:_(s32) = IMPLICIT_DEF
+...
+---
+# CHECK-LABEL: TestNoVRegsOverrideConflict
+# CHECK-SAME: has explicit property NoVRegs, but contains virtual registers
+name: TestNoVRegsOverrideConflict
+noVRegs: true
+body: |
+ bb.0:
+ %0:_(s32) = IMPLICIT_DEF
+...
diff --git a/llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties.mir b/llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties.mir
index 0cbdd8e1579430..858bbc8394bb34 100644
--- a/llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties.mir
+++ b/llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties.mir
@@ -1,9 +1,6 @@
# RUN: llc -run-pass none -o - %s | FileCheck %s
-# Test that we can disable certain properties that are normally computed. This
-# override is a soft-override (only allows overriding when it relaxes the
-# property). Therefore, a conflicting override (e.g. setting noPhis to true in
-# the presence of PHI nodes) should not result in an error, but instead should
-# use the computed property instead (noPhis = false in the mentioned example).
+
+# Test that we can disable certain properties that are normally computed
---
# CHECK-LABEL: name: TestNoPhis
@@ -26,21 +23,6 @@ name: TestNoPhisOverrideTrue
noPhis: true
...
---
-# CHECK-LABEL: name: TestNoPhisOverrideConflict
-# CHECK: noPhis: false
-# CHECK: ...
-name: TestNoPhisOverrideConflict
-noPhis: true
-tracksRegLiveness: true
-body: |
- bb.0:
- %0:_(s32) = IMPLICIT_DEF
-
- bb.1:
- %1:_(s32) = PHI %0, %bb.0, %0, %bb.1
- G_BR %bb.1
-...
----
# CHECK-LABEL: name: TestIsSSA
# CHECK: isSSA: true
# CHECK: ...
@@ -61,17 +43,6 @@ name: TestIsSSAOverrideTrue
isSSA: true
...
---
-# CHECK-LABEL: name: TestIsSSAOverrideConflict
-# CHECK: isSSA: false
-# CHECK: ...
-name: TestIsSSAOverrideConflict
-isSSA: true
-body: |
- bb.0:
- %0:_(s32) = IMPLICIT_DEF
- %0:_(s32) = IMPLICIT_DEF
-...
----
# CHECK-LABEL: name: TestNoVRegs
# CHECK: noVRegs: true
# CHECK: ...
@@ -91,13 +62,3 @@ noVRegs: false
name: TestNoVRegsOverrideTrue
noVRegs: true
...
----
-# CHECK-LABEL: name: TestNoVRegsOverrideConflict
-# CHECK: noVRegs: false
-# CHECK: ...
-name: TestNoVRegsOverrideConflict
-noVRegs: true
-body: |
- bb.0:
- %0:_(s32) = IMPLICIT_DEF
-...
>From 8b1b13b68e9b6976fea8dace6e7a8ef8a73b8531 Mon Sep 17 00:00:00 2001
From: Dominik Montada <dominik.montada at arm.com>
Date: Tue, 24 Sep 2024 08:35:50 +0200
Subject: [PATCH 5/6] Change setting logic, include full error message, use
G_IMPLICIT_DEF
---
llvm/lib/CodeGen/MIRParser/MIRParser.cpp | 21 +++----------------
...ptionally-computed-properties-conflict.mir | 17 +++++++--------
2 files changed, 10 insertions(+), 28 deletions(-)
diff --git a/llvm/lib/CodeGen/MIRParser/MIRParser.cpp b/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
index 8c0288d789216c..946e5db04ea68d 100644
--- a/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
+++ b/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
@@ -406,27 +406,12 @@ bool MIRParserImpl::computeFunctionProperties(
auto ComputedPropertyHelper =
[&Properties](std::optional<bool> ExplicitProp, bool ComputedProp,
MachineFunctionProperties::Property P) -> bool {
- // Prefer whatever is set explicitly by the input MIR
- if (ExplicitProp.has_value()) {
- if (*ExplicitProp) {
- // Check for conflicts with the computed value
- if (!ComputedProp)
- return true;
-
- Properties.set(P);
- } else
- Properties.reset(P);
-
- return false;
- }
-
- // No explicit value given, so use computed value
- if (ComputedProp)
+ if (ExplicitProp.value_or(ComputedProp))
Properties.set(P);
else
Properties.reset(P);
-
- return false;
+
+ return ExplicitProp && *ExplicitProp && !ComputedProp;
};
if (ComputedPropertyHelper(YamlMF.NoPHIs, !HasPHI,
diff --git a/llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties-conflict.mir b/llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties-conflict.mir
index 153a017d970e9a..d8d178d90ae0af 100644
--- a/llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties-conflict.mir
+++ b/llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties-conflict.mir
@@ -4,35 +4,32 @@
# properties
---
-# CHECK-LABEL: TestNoPhisOverrideConflict
-# CHECK-SAME: has explicit property NoPhi, but contains at least one PHI
+# CHECK: error: {{.*}}: TestNoPhisOverrideConflict has explicit property NoPhi, but contains at least one PHI
name: TestNoPhisOverrideConflict
noPhis: true
tracksRegLiveness: true
body: |
bb.0:
- %0:_(s32) = IMPLICIT_DEF
+ %0:_(s32) = G_IMPLICIT_DEF
bb.1:
%1:_(s32) = PHI %0, %bb.0, %1, %bb.1
G_BR %bb.1
...
---
-# CHECK-LABEL: TestIsSSAOverrideConflict
-# CHECK-SAME: has explicit property IsSSA, but is not valid SSA
+# CHECK: error: {{.*}}: TestIsSSAOverrideConflict has explicit property IsSSA, but is not valid SSA
name: TestIsSSAOverrideConflict
isSSA: true
body: |
bb.0:
- %0:_(s32) = IMPLICIT_DEF
- %0:_(s32) = IMPLICIT_DEF
+ %0:_(s32) = G_IMPLICIT_DEF
+ %0:_(s32) = G_IMPLICIT_DEF
...
---
-# CHECK-LABEL: TestNoVRegsOverrideConflict
-# CHECK-SAME: has explicit property NoVRegs, but contains virtual registers
+# CHECK: error: {{.*}}: TestNoVRegsOverrideConflict has explicit property NoVRegs, but contains virtual registers
name: TestNoVRegsOverrideConflict
noVRegs: true
body: |
bb.0:
- %0:_(s32) = IMPLICIT_DEF
+ %0:_(s32) = G_IMPLICIT_DEF
...
>From cf2ff04903efd900c533e29f6d217dcb462f271d Mon Sep 17 00:00:00 2001
From: Dominik Montada <dominik.montada at arm.com>
Date: Tue, 24 Sep 2024 08:54:15 +0200
Subject: [PATCH 6/6] Fix code formatting
---
llvm/lib/CodeGen/MIRParser/MIRParser.cpp | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/CodeGen/MIRParser/MIRParser.cpp b/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
index 946e5db04ea68d..8d6d800d761474 100644
--- a/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
+++ b/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
@@ -406,11 +406,13 @@ bool MIRParserImpl::computeFunctionProperties(
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;
};
More information about the llvm-commits
mailing list