> On Tue, Apr 09, 2024 at 07:07:58PM +1200, Thomas Munro wrote:
> On Sat, Apr 6, 2024 at 5:01 AM Dmitry Dolgov <9erthalion6@gmail.com> wrote:
> > > > Yep, I think this is it. I've spent some hours trying to understand why
> > > > suddenly deform function has noundef ret attribute, when it shouldn't --
> > > > this explains it and the proposed change fixes the crash. One thing that
> > > > is still not clear to me though is why this copied attribute doesn't
> > > > show up in the bitcode dumped right before running inline pass (I've
> > > > added this to troubleshoot the issue).
> > >
> > > One thing to consider in this context is indeed adding "verify" pass as
> > > suggested in the PR, at least for the debugging configuration. Without the fix
> > > it immediately returns:
> > >
> > > Running analysis: VerifierAnalysis on deform_0_1
> > > Attribute 'noundef' applied to incompatible type!
> > >
> > > llvm error: Broken function found, compilation aborted!
> >
> > Here is what I have in mind. Interestingly enough, it also shows few
> > more errors besides "noundef":
> >
> > Intrinsic name not mangled correctly for type arguments! Should be: llvm.lifetime.end.p0
> > ptr @llvm.lifetime.end.p0i8
> >
> > It refers to the function from create_LifetimeEnd, not sure how
> > important is this.
>
> Would it be too slow to run the verify pass always, in assertion
> builds? Here's a patch for the original issue, and a patch to try
> that idea + a fix for that other complaint it spits out. The latter
> would only run for LLVM 17+, but that seems OK.
Sounds like a good idea. About the overhead, I've done a quick test on the
reproducer at hands, doing explain analyze in a tight loop and fetching
"optimization" timinigs. It gives quite visible difference 96ms p99 with verify
vs 46ms p99 without verify (and a rather low stddev, ~1.5ms). But I can
imagine it's acceptable for a build with assertions.
Btw, I've found there is a C-api for this exposed, which produces the same
warnings for me. Maybe it would be even better this way:
/**
* Toggle adding the VerifierPass for the PassBuilder, ensuring all functions
* inside the module is valid.
*/
void LLVMPassBuilderOptionsSetVerifyEach(LLVMPassBuilderOptionsRef Options,
LLVMBool VerifyEach);
+ /* In assertion builds, run the LLVM verify pass. */
+#ifdef USE_ASSERT_CHECKING
+ LLVMPassBuilderOptionsSetVerifyEach(options, true);
+#endif