[PATCH] D111397: RFC: A new way of skipping machine verification after problematic passes
Matthias Braun via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 12 09:35:56 PDT 2021
MatzeB added a comment.
As discussed in https://reviews.llvm.org/D111386 I have a feeling that some targets can likely be improved by moving code from `addPreEmitPass` to `addPreEmitPass2`. But we should probably discuss this independently of this patch, since I have a feeling that one way or another we won't end up with 100% of things fixed.
I am in favor of this change and just have some nitpicks.
================
Comment at: llvm/include/llvm/CodeGen/MachineFunction.h:152-166
+ // Verifies: Means that the function is expected to pass machine verification.
+ // This can be cleared by passes that introduce known problems that have not
+ // been fixed yet.
enum class Property : unsigned {
IsSSA,
NoPHIs,
TracksLiveness,
----------------
Can we use a flag where the default/desired state is not set?
================
Comment at: llvm/lib/CodeGen/MachineFunction.cpp:160
Properties.set(MachineFunctionProperties::Property::TracksLiveness);
+ Properties.set(MachineFunctionProperties::Property::Verifies);
if (STI->getRegisterInfo())
----------------
See above (when switching to `DisableVerification`).
================
Comment at: llvm/lib/CodeGen/MachineVerifier.cpp:296-297
+ // Skip functions that have known verification problems.
+ // FIXME: Remove this mechanism when all problematic passes have been
+ // fixed.
+ if (!MF.getProperties().hasProperty(
----------------
FWIW: I don't have high hopes for this flag ever disappearing once we introduce it and would tend to drop the comment (but no strong opinion you can keep it if you want).
================
Comment at: llvm/lib/Target/AMDGPU/AMDILCFGStructurizer.cpp:130
bool runOnMachineFunction(MachineFunction &MF) override {
+ // FIXME: This pass causes verification failures.
+ MF.getProperties().reset(MachineFunctionProperties::Property::Verifies);
----------------
So, there's this thing I've seen other projects do, where they file tickets in the bugtracker and reference those in code. I know we don't usually do this in LLVM yet, but maybe we can set a new precedent here, what do you think?
================
Comment at: llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp:790
bool SILowerControlFlow::runOnMachineFunction(MachineFunction &MF) {
+ // FIXME: This pass causes verification failures.
+ MF.getProperties().reset(MachineFunctionProperties::Property::Verifies);
----------------
As above.
================
Comment at: llvm/lib/Target/Hexagon/HexagonVLIWPacketizer.cpp:206
bool HexagonPacketizer::runOnMachineFunction(MachineFunction &MF) {
+ // FIXME: This pass causes verification failures.
+ MF.getProperties().reset(MachineFunctionProperties::Property::Verifies);
----------------
as above
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111397/new/
https://reviews.llvm.org/D111397
More information about the llvm-commits
mailing list