<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jan 8, 2021 at 8:43 AM Nikita Popov <<a href="mailto:nikita.ppv@gmail.com">nikita.ppv@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jan 8, 2021 at 2:31 PM Sanjay Patel via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jan 8, 2021 at 8:19 AM Roman Lebedev <<a href="mailto:lebedev.ri@gmail.com" target="_blank">lebedev.ri@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Fri, Jan 8, 2021 at 4:10 PM Sanjay Patel via llvm-commits<br>
<<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
><br>
><br>
> Author: Sanjay Patel<br>
> Date: 2021-01-08T08:10:17-05:00<br>
> New Revision: 267ff7901c745dc903d55599240464ebc4c0bda3<br>
><br>
> URL: <a href="https://github.com/llvm/llvm-project/commit/267ff7901c745dc903d55599240464ebc4c0bda3" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/267ff7901c745dc903d55599240464ebc4c0bda3</a><br>
> DIFF: <a href="https://github.com/llvm/llvm-project/commit/267ff7901c745dc903d55599240464ebc4c0bda3.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/267ff7901c745dc903d55599240464ebc4c0bda3.diff</a><br>
><br>
> LOG: [SLP] limit verifyFunction to debug build (PR48689)<br>
><br>
> As noted in PR48689, the verifier may have some kind<br>
> of exponential behavior that should be addressed<br>
> separately. For now, only run it in debug mode to<br>
> prevent problems for release+asserts.<br>
> That limit is what we had before D80401, and I'm<br>
> not sure if there was a reason to change it in that<br>
> patch.<br>
><br>
> Added:<br>
><br>
><br>
> Modified:<br>
>     llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp<br>
><br>
> Removed:<br>
><br>
><br>
><br>
> ################################################################################<br>
> diff  --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp<br>
> index 8c06e29341ad..ef0dea0f11d3 100644<br>
> --- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp<br>
> +++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp<br>
> @@ -2499,7 +2499,7 @@ BoUpSLP::~BoUpSLP() {<br>
>             "trying to erase instruction with users.");<br>
>      Pair.getFirst()->eraseFromParent();<br>
>    }<br>
> -  assert(!verifyFunction(*F, &dbgs()));<br>
> +  LLVM_DEBUG(verifyFunction(*F));<br>
Nitpick: `assert()` already limited the code to debug build only (well,<br>
-DLLVM_ENABLE_ASSERTIONS=ON) Wrapping it into LLVM_DEBUG()<br>
made it run wherever `-mllvm -debug-only=SLP` is specified.<br></blockquote><div><br></div><div>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.</div></div></div></blockquote><div><br></div><div>Shouldn't this go under EXPENSIVE_CHECKS, rather than making it debug-only?<br></div></div></div></blockquote><div><br></div><div>Hmm, yes that's another option. I don't have a good sense of what the common pattern is. Should all calls to verifyFunction require EXPENSIVE_CHECKS (if so, we would put the ifdef inside the call)? <br></div><div> </div></div></div>