[PATCH] D33696: TargetMachine: Indicate whether machine verifier passes.

Matthias Braun via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 31 10:34:37 PDT 2017


MatzeB added a comment.



In https://reviews.llvm.org/D33696#768678, @RKSimon wrote:

> x86 looks good.
>
> Probably the most pragmatic solution for now, I'm just concerned that this will lead to some targets never getting fixed......


Indeed I also have some worries that that callback will never go away once introduced. However I find the status quo where pretty much all targets are broken even more worrying and I am optimistic that this function acts more as a motivation and goal when people see that other targets are clean and theirs isn't.

In https://reviews.llvm.org/D33696#768657, @filcab wrote:

> Should we add TODO comments to all those overrides and state that the errors should be fixed and the overrides (and the function itself, afterwards) be removed?


I will add a comment that the function is a stopgap measure and will be removed. My plan for the targets is to file PRs against them which I hope works better than TODOs in the source.



================
Comment at: lib/Target/AMDGPU/AMDGPUTargetMachine.h:73-75
+  bool isMachineVerifierClean() const override {
+    return false;
+  }
----------------
tstellar wrote:
> I think GCNTargetMachine should be able to return true for this, did you try this?
No I have not tested the individual AMDGPU target machines, I'll try.


Repository:
  rL LLVM

https://reviews.llvm.org/D33696





More information about the llvm-commits mailing list