On Tue, 28 May 2024, Richard Biener wrote:

> On Wed, May 15, 2024 at 12:59 PM Alexander Monakov <amona...@ispras.ru> wrote:
> >
> >
> > Hello,
> >
> > I'd like to ask if anyone has any new thoughts on this patch.
> >
> > Let me also point out that valgrind/memcheck.h is permissively
> > licensed (BSD-style, rest of Valgrind is GPLv2), with the intention
> > to allow importing into projects that are interested in using
> > client requests without build-time dependency on installed headers.
> > So maybe we have that as an option too.
> 
> Inlining the VALGRIND_DO_CLIENT_REQUEST_EXPR would be a lot
> cheaper

I am a bit confused what you mean by "cheaper". Could it be that we are not
on the same page regarding the machine code behind client requests?

Here's how the proposed libgcc helper compiles on amd64:

        ; Prepare the descriptor structure on the stack
        movq    $1296236545, -56(%rsp)
        leaq    -56(%rsp), %rax
        xorl    %edx, %edx
        movq    %rdi, -48(%rsp)
        movq    %rsi, -40(%rsp)
        movq    $0, -32(%rsp)
        movq    $0, -24(%rsp)
        movq    $0, -16(%rsp)
#APP
        ; Request preamble: Valgrind detects the useless
        ; rotate sequence. Other architectures use different
        ; preambles.
        rolq $3,  %rdi ; rolq $13, %rdi
        rolq $61, %rdi ; rolq $51, %rdi
        ; Request trigger (also varies by target).
        xchgq %rbx,%rbx
#NO_APP
        ; The (ignored) result is a volatile automatic variable
        movq    %rdx, -64(%rsp)
        movq    -64(%rsp), %rax

I think this is not well-suited for inlining, and to expand it you need to know
the layout of the descriptor struct and machine-specific preamble+trigger.

Also I am not sure where that would leave us with target support. What if
folks will be eventually interested in using this to hunt down mysterious
miscompilations on, say, PowerPC?
 
> and would not add to the libgcc ABI.

What about linking a new library with that helper?

> I would guess the valgrind "ABI" for these is practically fixed but of course
> architecture dependent.
> 
> I do like the feature in general.

Thank you.
Alexander

Reply via email to