[MIR] Allow overriding isSSA, noPhis, noVRegs in MIR input #108546
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-regalloc @llvm/pr-subscribers-backend-hexagon Author: Dominik Montada (gargaroff) ChangesAllow 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 Full diff: https://github.com/llvm/llvm-project/pull/108546.diff 11 Files Affected:
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 a5d6a40392d0cb..7d0876a33b70d4 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 6e23969cd99bac..eb5259fd4b77ba 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 0b1fdf9c33d66c..b196d13deb74ac 100644
--- a/llvm/test/CodeGen/AArch64/mlicm-stack-write-check.mir
+++ b/llvm/test/CodeGen/AArch64/mlicm-stack-write-check.mir
@@ -2,6 +2,7 @@
---
name: test
tracksRegLiveness: true
+isSSA: false
registers:
- { id: 0, class: gpr64 }
stack:
@@ -29,11 +30,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:
@@ -61,5 +62,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
|
arsenm
reviewed
Sep 13, 2024
llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties.mir
Show resolved
Hide resolved
|
@arsenm Ping |
|
We do need these explicit overrides for some MIR tests, but the description is imprecise:
Passes that insert phis should be clearing the no phis property |
arsenm
reviewed
Sep 20, 2024
llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties.mir
Outdated
Show resolved
Hide resolved
llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties.mir
Outdated
Show resolved
Hide resolved
llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties.mir
Outdated
Show resolved
Hide resolved
qcolombet
reviewed
Sep 20, 2024
MatzeB
approved these changes
Sep 20, 2024
There was a problem hiding this comment.
I guess it's fair enough to add the functionality. Though judging from the tests one can blame some of the problems to the passes:
- If
FinalizeISelpass is (potentially) creating PHI instructions, then it should probably set the noPhis property to false MachineLICMdecidingPreRegAlloc = MRI->isSSA();is probably not great either and maybe we should have two separate pass names for the two use cases or something instead of takingSSAas an indicator whether regalloc has happened...
but that may be for another time to clean up...
Anyway probably still a good idea to allow explicitely setting the flags and overriding the computation.
So LGTM
llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties.mir
Outdated
Show resolved
Hide resolved
That sounds like an unnecessary burden on passes that create instructions. Surely the only supported use case is that functions start off with noPhis=false even if they happen to contain zero phi instructions, and after phi elimination they have noPhis=true. Any pass that might create phis simply should not be run after that point. |
arsenm
reviewed
Sep 23, 2024
llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties-conflict.mir
Outdated
Show resolved
Hide resolved
llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties-conflict.mir
Outdated
Show resolved
Hide resolved
arsenm
approved these changes
Sep 24, 2024
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
Who is responsible for clicking the |
It doesn't matter, I prefer if you just do it |
arsenm
approved these changes
Sep 24, 2024
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 llvm#37787
gargaroff
force-pushed
the
gargaroff/override_computed_properties
branch
from
6b805be to
cf2ff04
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/16/builds/5941 Here is the relevant piece of the build log for the reference |
also https://lab.llvm.org/buildbot/#/builders/187/builds/1416 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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