[PATCH] D111397: Add new MachineFunction property FailsVerification

Fri Oct 15 03:44:12 PDT 2021

foad marked 3 inline comments as done.
foad added a comment.

In D111397#3058959 <https://reviews.llvm.org/D111397#3058959>, @MatzeB wrote:

> In D111397#3050710 <https://reviews.llvm.org/D111397#3050710>, @foad wrote:
>
>> Known issues:
>>
>> 1. I'm not sure if the new "Verifies" property needs to be serialised in .mir files.
>
> Yes I think so.

Done.

================
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(
----------------
MatzeB wrote:
> 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).
I'm an optimist!

================
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);
----------------
MatzeB wrote:
> 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?
I'm happy to do this in principle.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111397/new/

https://reviews.llvm.org/D111397