Hi,
On 2024-04-10 22:15:27 +0200, Dmitry Dolgov wrote:
> > On Wed, Apr 10, 2024 at 12:43:23PM +1200, Thomas Munro wrote:
> > On Tue, Apr 9, 2024 at 10:05 PM Dmitry Dolgov <9erthalion6@gmail.com> wrote:
> > > + /* In assertion builds, run the LLVM verify pass. */
> > > +#ifdef USE_ASSERT_CHECKING
> > > + LLVMPassBuilderOptionsSetVerifyEach(options, true);
> > > +#endif
> >
> > Thanks, that seems nicer. I think the question is whether it will
> > slow down build farm/CI/local meson test runs to a degree that exceeds
> > its value. Another option would be to have some other opt-in macro,
> > like the existing #ifdef LLVM_PASS_DEBUG, for people who maintain
> > JIT-related stuff to turn on.
>
> Oh, I see. I'm afraid I don't have enough knowledge of the CI pipeline,
> but at least locally for me installcheck became only few percent slower
> with the verify pass.
I think it's worthwhile to add. It makes some problems so much easier to
find. And if you're building with debug enabled llvm, performance is so
atrocious anyway, that this isn't going to change the big picture...
> > Supposing we go with USE_ASSERT_CHECKING, I have another question:
> >
> > - const char *nm = "llvm.lifetime.end.p0i8";
> > + const char *nm = "llvm.lifetime.end.p0";
> >
> > Was that a mistake, or did the mangling rules change in some version?
>
> I'm not sure, but inclined to think it's the same as with noundef -- a
> mistake, which was revealed in some recent version of LLVM. From what I
> understand the suffix i8 indicates an overloaded argument of that type,
> which is probably not needed. At the same time I can't get this error
> from the verify pass with LLVM-12 or LLVM-15 (I have those at hand by
> accident). To make it even more confusing I've found a few similar
> examples in other projects, where this was really triggered by an issue
> in LLVM [1].
I'm afraid that it actually has changed over time, I'm fairly sure that it was
required at some point.
Greetings,
Andres Freund