[PATCH] D77576: [globalisel] Add lost debug locations verifier

Wed Apr 15 16:35:50 PDT 2020

dsanders added a comment.

In D77576#1979013 <https://reviews.llvm.org/D77576#1979013>, @dsanders wrote:

> In D77576#1978914 <https://reviews.llvm.org/D77576#1978914>, @qcolombet wrote:
>
> > Hi Daniel,
> >
> > What is the compile time impact of this change?
>
>
> That depends on VerifyDebugLocs. When it's `None` (default for release), there shouldn't be a measurable impact as the observer doesn't get installed. I haven't measured it for `Locations` (current default for asserts) yet as I've been more focused on fixing the bugs it finds in our out-of-tree target so far. I don't expect it to be noticable as it's a fairly simple matcher on small search spaces but if it is we can change the default to None at the cost of needing to use an extra option in out `llvm-lit -Dllc='llc ...'` command.

Running CTMark with -j20 (because I'm not looking at individual tests, just the rough total), I get:

- X86 -fglobal-isel -g: 3215s without this patch, 3209s with it
- X86 -fglobal-isel: 3133s without this patch, 3133s with it
- AArch64 -fglobal-isel -g: 3065s without this patch, 3053s with it
- AArch64 -fglobal-isel: 2972s without this patch, 2979s with it

So I'd say that leaving it on by default for asserts has no significant impact on compile time

I did run into an interesting issue in some other testing I was doing though. A verifier failure can cause GlobalISel to fall back on DAGISel when fallbacks are enabled, changing codegen. I'm going to have to tweak how failures are reported to fix that

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77576