From: Eduard Zingerman <eddyz87@gmail.com>
To: Alan Maguire <alan.maguire@oracle.com>,
acme@kernel.org, yhs@fb.com, ast@kernel.org, olsajiri@gmail.com,
timo@incline.eu
Cc: daniel@iogearbox.net, andrii@kernel.org, songliubraving@fb.com,
john.fastabend@gmail.com, kpsingh@chromium.org, sdf@google.com,
haoluo@google.com, martin.lau@kernel.org, bpf@vger.kernel.org
Subject: Re: [PATCH dwarves 1/5] dwarves: help dwarf loader spot functions with optimized-out parameters
Date: Thu, 26 Jan 2023 02:20:01 +0200 [thread overview]
Message-ID: <9ed96849303fbc3dee1da5dccac05bd11fb04789.camel@gmail.com> (raw)
In-Reply-To: <bc50242da1ea8b3b3eafb62e880ed4a278492d2c.camel@gmail.com>
On Thu, 2023-01-26 at 01:42 +0200, Eduard Zingerman wrote:
> On Wed, 2023-01-25 at 22:52 +0000, Alan Maguire wrote:
> [...]
> >
> > Thanks for this - I tried it, and we spot the optimization once we update
> > die__create_new_parameter() as follows:
> >
> > diff --git a/dwarf_loader.c b/dwarf_loader.c
> > index f96b6ff..605ad45 100644
> > --- a/dwarf_loader.c
> > +++ b/dwarf_loader.c
> > @@ -1529,6 +1530,8 @@ static struct tag *die__create_new_parameter(Dwarf_Die *di
> >
> > if (ftype != NULL) {
> > ftype__add_parameter(ftype, parm);
> > + if (parm->optimized)
> > + ftype->optimized_parms = 1;
> > if (param_idx >= 0) {
> > if (add_child_llvm_annotations(die, param_idx, conf, &(t
> > return NULL;
> >
>
> Great, looks good.
>
> > With that change, I see:
> >
> > $ pahole --verbose --btf_encode_detached=test.btf test.o
> > btf_encoder__new: 'test.o' doesn't have '.data..percpu' section
> > Found 0 per-CPU variables!
> > Found 2 functions!
> > File test.o:
> > [1] INT int size=4 nr_bits=32 encoding=SIGNED
> > [2] PTR (anon) type_id=3
> > [3] PTR (anon) type_id=4
> > [4] INT char size=1 nr_bits=8 encoding=SIGNED
> > [5] FUNC_PROTO (anon) return=1 args=(1 argc, 2 argv)
> > [6] FUNC main type_id=5
> > added local function 'f', optimized-out params
> > skipping addition of 'f' due to optimized-out parameters
>
> Sorry, I have one more silly program.
>
> I talked to Yonghong today and we discussed if compiler can change a
> type of a function parameter as a result of some optimization.
> Consider the following example:
>
> $ cat test.c
> struct st {
> int a;
> int b;
> };
>
> __attribute__((noinline))
> static int f(struct st *s) {
> return s->a + s->b;
> }
>
> int main(int argc, char *argv[]) {
> struct st s = {
> .a = (long)argv[0],
> .b = (long)argv[1]
> };
> return f(&s);
> }
>
> When compiled by `clang` with -O3 the prototype of `f` is changed from
> `int f(struct *st)` to `int f(int, int)`:
>
> $ clang -O3 -g -c test.c -o test.o && llvm-objdump -d test.o
> ...
> 0000000000000000 <main>:
> 0: 8b 3e movl (%rsi), %edi
> 2: 8b 76 08 movl 0x8(%rsi), %esi
> 5: eb 09 jmp 0x10 <f>
> 7: 66 0f 1f 84 00 00 00 00 00 nopw (%rax,%rax)
>
> 0000000000000010 <f>:
> 10: 8d 04 37 leal (%rdi,%rsi), %eax
> 13: c3 retq
>
> But generated DWARF hides this information:
Actually, I'm not correct. The information is present because
`DW_AT_location` attribute is not present (just as 4.1.4 says).
So I think that the condition for optimized parameters detection has
to be adjusted one more time:
has_location = attr_location(die, &loc.expr, &loc.exprlen) == 0;
has_const_value = dwarf_attr(die, DW_AT_const_value, &attr) != NULL;
if (has_location && loc.exprlen != 0) {
Dwarf_Op *expr = loc.expr;
switch (expr->atom) {
case DW_OP_reg1 ... DW_OP_reg31:
case DW_OP_breg0 ... DW_OP_breg31:
break;
default:
parm->optimized = true;
break;
}
} else if (!has_location || has_const_value) {
parm->optimized = true;
}
(But again, the parameter is marked as optimized but the function is
not skipped in the final BTF, so either I applied our last change
incorrectly or something additional should be done).
wdyt?
> $ llvm-dwarfdump test.o
> ...
> 0x0000005c: DW_TAG_subprogram
> DW_AT_low_pc (0x0000000000000010)
> DW_AT_high_pc (0x0000000000000014)
> DW_AT_frame_base (DW_OP_reg7 RSP)
> DW_AT_call_all_calls (true)
> DW_AT_name ("f")
> DW_AT_decl_file ("/home/eddy/work/tmp/test.c")
> DW_AT_decl_line (7)
> DW_AT_prototyped (true)
> DW_AT_type (0x00000074 "int")
>
> 0x0000006b: DW_TAG_formal_parameter
> DW_AT_name ("s")
> DW_AT_decl_file ("/home/eddy/work/tmp/test.c")
> DW_AT_decl_line (7)
> DW_AT_type (0x0000009e "st *")
>
> 0x00000073: NULL
> ...
>
> Is this important?
> (gcc does not do this for the particular example, but I don't know if
> it could be tricked to under some conditions).
>
> Thanks,
> Eduard
>
> [...]
next prev parent reply other threads:[~2023-01-26 0:20 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-24 13:45 [PATCH dwarves 0/5] dwarves: support encoding of optimized-out parameters, removal of inconsistent static functions Alan Maguire
2023-01-24 13:45 ` [PATCH dwarves 1/5] dwarves: help dwarf loader spot functions with optimized-out parameters Alan Maguire
2023-01-25 16:53 ` Jiri Olsa
2023-01-25 17:47 ` Eduard Zingerman
2023-01-25 18:28 ` Alan Maguire
2023-01-25 21:34 ` Eduard Zingerman
2023-01-25 22:52 ` Alan Maguire
2023-01-25 23:42 ` Eduard Zingerman
2023-01-26 0:20 ` Eduard Zingerman [this message]
2023-01-26 14:02 ` Alan Maguire
2023-01-26 15:02 ` Eduard Zingerman
2023-01-24 13:45 ` [PATCH dwarves 2/5] btf_encoder: refactor function addition into dedicated btf_encoder__add_func Alan Maguire
2023-01-24 13:45 ` [PATCH dwarves 3/5] btf_encoder: child encoders should have a reference to parent encoder Alan Maguire
2023-01-24 13:45 ` [PATCH dwarves 4/5] btf_encoder: represent "."-suffixed optimized functions (".isra.0") in BTF Alan Maguire
2023-01-25 17:54 ` Kui-Feng Lee
2023-01-25 18:56 ` Arnaldo Carvalho de Melo
2023-01-26 18:37 ` Kui-Feng Lee
2023-01-25 18:59 ` Alan Maguire
2023-01-26 17:43 ` Kui-Feng Lee
2023-01-24 13:45 ` [PATCH dwarves 5/5] btf_encoder: skip BTF encoding of static functions with inconsistent prototypes Alan Maguire
2023-01-25 13:39 ` Jiri Olsa
2023-01-25 14:18 ` Alan Maguire
2023-01-25 16:53 ` Jiri Olsa
2023-01-26 14:12 ` Alan Maguire
2023-01-24 15:14 ` [PATCH dwarves 0/5] dwarves: support encoding of optimized-out parameters, removal of inconsistent static functions Jiri Olsa
2023-01-24 16:11 ` Alan Maguire
2023-01-25 13:59 ` Jiri Olsa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9ed96849303fbc3dee1da5dccac05bd11fb04789.camel@gmail.com \
--to=eddyz87@gmail.com \
--cc=acme@kernel.org \
--cc=alan.maguire@oracle.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=kpsingh@chromium.org \
--cc=martin.lau@kernel.org \
--cc=olsajiri@gmail.com \
--cc=sdf@google.com \
--cc=songliubraving@fb.com \
--cc=timo@incline.eu \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox