[llvm] 267ff79 - [SLP] limit verifyFunction to debug build (PR48689)

[Nikita Popov via llvm-commits]

On Fri, Jan 8, 2021 at 2:31 PM Sanjay Patel via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

>
>
> On Fri, Jan 8, 2021 at 8:19 AM Roman Lebedev <lebedev.ri at gmail.com> wrote:
>
>> On Fri, Jan 8, 2021 at 4:10 PM Sanjay Patel via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>> >
>> >
>> > Author: Sanjay Patel
>> > Date: 2021-01-08T08:10:17-05:00
>> > New Revision: 267ff7901c745dc903d55599240464ebc4c0bda3
>> >
>> > URL:
>> https://github.com/llvm/llvm-project/commit/267ff7901c745dc903d55599240464ebc4c0bda3
>> > DIFF:
>> https://github.com/llvm/llvm-project/commit/267ff7901c745dc903d55599240464ebc4c0bda3.diff
>> >
>> > LOG: [SLP] limit verifyFunction to debug build (PR48689)
>> >
>> > As noted in PR48689, the verifier may have some kind
>> > of exponential behavior that should be addressed
>> > separately. For now, only run it in debug mode to
>> > prevent problems for release+asserts.
>> > That limit is what we had before D80401, and I'm
>> > not sure if there was a reason to change it in that
>> > patch.
>> >
>> > Added:
>> >
>> >
>> > Modified:
>> >     llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
>> >
>> > Removed:
>> >
>> >
>> >
>> >
>> ################################################################################
>> > diff  --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
>> b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
>> > index 8c06e29341ad..ef0dea0f11d3 100644
>> > --- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
>> > +++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
>> > @@ -2499,7 +2499,7 @@ BoUpSLP::~BoUpSLP() {
>> >             "trying to erase instruction with users.");
>> >      Pair.getFirst()->eraseFromParent();
>> >    }
>> > -  assert(!verifyFunction(*F, &dbgs()));
>> > +  LLVM_DEBUG(verifyFunction(*F));
>> Nitpick: `assert()` already limited the code to debug build only (well,
>> -DLLVM_ENABLE_ASSERTIONS=ON) Wrapping it into LLVM_DEBUG()
>> made it run wherever `-mllvm -debug-only=SLP` is specified.
>>
>
> Yes. IIUC, there's a (potentially large) fraction of LLVM users that build
> as release with asserts and expect release-like behavior (good perf), so we
> want to maintain that mode of operation. Once we go to "full" debug build,
> we give up on any hope of having a useful experience for anything but
> full-on (extremely slow) debugging.
>

Shouldn't this go under EXPENSIVE_CHECKS, rather than making it debug-only?

Nikita
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210108/3cba23c9/attachment.html>