All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiong Wang <jiong.wang@netronome.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>,
	Jiong Wang <jiong.wang@netronome.com>,
	daniel@iogearbox.net, bpf@vger.kernel.org,
	netdev@vger.kernel.org, oss-drivers@netronome.com
Subject: Re: [PATCH v4 bpf-next 02/15] bpf: mark lo32 writes that should be zero extended into hi32
Date: Sat, 20 Apr 2019 00:27:05 +0100	[thread overview]
Message-ID: <ly8sw5bmpi.fsf@netronome.com> (raw)
In-Reply-To: <20190419214121.2rbmmms6qozmiuke@ast-mbp.dhcp.thefacebook.com>


Alexei Starovoitov writes:

> On Fri, Apr 19, 2019 at 02:33:10PM -0700, Jakub Kicinski wrote:
>> On Fri, 19 Apr 2019 14:14:05 -0700, Alexei Starovoitov wrote:
>> > > This reminds me, I'm not entirely clear on the need to propagate the
>> > > zext through stack slots...  Pointers are guaranteed to be 64bit, we
>> > > don't save parentage on scalars (AFAICT),  
>> > 
>> > scalars have parentage chain too.
>> > we don't track them precisely when they're spilled to stack.
>> > That actually caused an issue recently when valid program was rejected,
>> > so we might add a feature to track full contents of scalars in the stack.
>> 
>> Interesting..
>> 
>> > > why not pass REG_LIVE_READ
>> > > or READ64 to mark_reg_read() from stack_read?  
>> > 
>> > can we agree on only two states first ? ;)
>> 
>> Yess, the LIVE_READ was thought to be more of a mask for those accesses
>> that only care about "any read" being set, to be honest.  As you said
>> read64 is a strict superset of read32.  Keeping the name REG_LIVE_READ,
>> rather than REG_LIVE_READ_ANY or _MASK let us leave some of the
>> existing code untouched.
>> 
>> Jiong's original idea was to add a read32, and have read mean read64.
>> 
>> I think you said we should have read32 and read64 flags, but clear
>> read32 once read64 gets set?  SGTM!
>
> yep.
>
> any subsequent read64 means that earlier read32 marks are irrelevant
> from zext optimization pov.

OK, will split REG_LIVE_READ into REG_LIVE_READ64 and REG_LIVE_READ32, and
will let the prior override the latter early inside mark_reg_read. I feel
renaming parameter for propagate_liveness (the "parent" etc) could be a
following up patch? Let me know if you want it included in this set.

(I am travelling, will try to send out updated version around next Tuesday)

Regards,
Jiong

  reply	other threads:[~2019-04-19 23:27 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-15 17:26 [PATCH v4 bpf-next 00/15] bpf: eliminate zero extensions for sub-register writes Jiong Wang
2019-04-15 17:26 ` [PATCH v4 bpf-next 01/15] bpf: split read liveness into REG_LIVE_READ64 and REG_LIVE_READ32 Jiong Wang
2019-04-15 23:03   ` Jakub Kicinski
2019-04-16  1:26   ` Alexei Starovoitov
2019-04-16  7:39     ` Jiong Wang
2019-04-16 16:20       ` Alexei Starovoitov
2019-04-16 20:19         ` Jiong Wang
2019-04-15 17:26 ` [PATCH v4 bpf-next 02/15] bpf: mark lo32 writes that should be zero extended into hi32 Jiong Wang
2019-04-18 23:57   ` Alexei Starovoitov
2019-04-19 20:40     ` Jakub Kicinski
2019-04-19 21:14       ` Alexei Starovoitov
2019-04-19 21:33         ` Jakub Kicinski
2019-04-19 21:41           ` Alexei Starovoitov
2019-04-19 23:27             ` Jiong Wang [this message]
2019-04-19 23:28               ` Alexei Starovoitov
2019-04-15 17:26 ` [PATCH v4 bpf-next 03/15] bpf: reduce false alarm by refining helper call arg types Jiong Wang
2019-04-15 17:26 ` [PATCH v4 bpf-next 04/15] bpf: insert explicit zero extension insn when hardware doesn't do it implicitly Jiong Wang
2019-04-15 17:26 ` [PATCH v4 bpf-next 05/15] bpf: introduce new bpf prog load flags "BPF_F_TEST_RND_HI32" Jiong Wang
2019-04-15 17:26 ` [PATCH v4 bpf-next 06/15] bpf: randomize high 32-bit when BPF_F_TEST_RND_HI32 is set Jiong Wang
2019-04-15 17:26 ` [PATCH v4 bpf-next 07/15] libbpf: add "prog_flags" to bpf_program/bpf_prog_load_attr/bpf_load_program_attr Jiong Wang
2019-04-15 17:26 ` [PATCH v4 bpf-next 08/15] selftests: enable hi32 randomization for all tests Jiong Wang
2019-04-15 17:26 ` [PATCH v4 bpf-next 09/15] arm: bpf: eliminate zero extension code-gen Jiong Wang
2019-04-15 17:26 ` [PATCH v4 bpf-next 10/15] powerpc: " Jiong Wang
2019-04-15 17:26 ` [PATCH v4 bpf-next 11/15] s390: " Jiong Wang
2019-04-15 17:26 ` [PATCH v4 bpf-next 12/15] sparc: " Jiong Wang
2019-04-15 17:26 ` [PATCH v4 bpf-next 13/15] x32: " Jiong Wang
2019-04-15 17:26 ` [PATCH v4 bpf-next 14/15] riscv: " Jiong Wang
2019-04-17  7:55   ` Björn Töpel
2019-04-15 17:26 ` [PATCH v4 bpf-next 15/15] nfp: " Jiong Wang
2019-04-24 16:31   ` kbuild test robot

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=ly8sw5bmpi.fsf@netronome.com \
    --to=jiong.wang@netronome.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=jakub.kicinski@netronome.com \
    --cc=netdev@vger.kernel.org \
    --cc=oss-drivers@netronome.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.