From 8ba334bc4ad1e20c8201b85ed0a3e3b87bc47fe1 Mon Sep 17 00:00:00 2001 From: Dominik Montada Date: Tue, 24 Sep 2024 14:21:45 +0200 Subject: [PATCH] [MIR] Allow overriding isSSA, noPhis, noVRegs in MIR input (#108546) 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 #37787 --- llvm/include/llvm/CodeGen/MIRYamlMapping.h | 11 ++++ llvm/lib/CodeGen/MIRParser/MIRParser.cpp | 52 +++++++++++---- llvm/lib/CodeGen/MIRPrinter.cpp | 7 ++ .../AArch64/mlicm-stack-write-check.mir | 4 +- .../Hexagon/expand-condsets-impuse2.mir | 2 +- .../Hexagon/expand-condsets-phys-reg.mir | 2 +- .../Hexagon/expand-condsets-rm-reg.mir | 2 +- ...ptionally-computed-properties-conflict.mir | 35 ++++++++++ ...unction-optionally-computed-properties.mir | 64 +++++++++++++++++++ .../X86/sjlj-shadow-stack-liveness.mir | 3 +- .../llvm-reduce/mir/preserve-func-info.mir | 6 ++ 11 files changed, 170 insertions(+), 18 deletions(-) create mode 100644 llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties-conflict.mir 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..ab8dc442e04b7b 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 + std::optional NoPHIs; + std::optional IsSSA; + std::optional NoVRegs; + bool CallsEHReturn = false; bool CallsUnwindInit = false; bool HasEHCatchret = false; @@ -770,6 +775,12 @@ template <> struct MappingTraits { 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()); + YamlIO.mapOptional("isSSA", MF.IsSSA, std::optional()); + YamlIO.mapOptional("noVRegs", MF.NoVRegs, std::optional()); + 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..8d6d800d761474 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); + bool 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) { +bool MIRParserImpl::computeFunctionProperties( + MachineFunction &MF, const yaml::MachineFunction &YamlMF) { MachineFunctionProperties &Properties = MF.getProperties(); bool HasPHI = false; @@ -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 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( @@ -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; 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/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-conflict.mir b/llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties-conflict.mir new file mode 100644 index 00000000000000..d8d178d90ae0af --- /dev/null +++ b/llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties-conflict.mir @@ -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 +... 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..858bbc8394bb34 --- /dev/null +++ b/llvm/test/CodeGen/MIR/Generic/machine-function-optionally-computed-properties.mir @@ -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 +... 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