From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qv1-xf2f.google.com (mail-qv1-xf2f.google.com [IPv6:2607:f8b0:4864:20::f2f]) by sourceware.org (Postfix) with ESMTPS id 055663858C52 for ; Mon, 12 Feb 2024 22:55:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 055663858C52 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=obs.cr Authentication-Results: sourceware.org; spf=none smtp.mailfrom=obs.cr ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 055663858C52 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::f2f ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707778540; cv=none; b=uT8jSAG6qwTsGJvB1jSqEtXjqkXZHgh8fwA3tUXcJWbAGSZalurcwoMLgbz5C2MViNq4G/vt/Ih+OpnEyRVgZmhj6E3YiKRiEQrK2kvDgeKT8FxmEBeDUQC+3FAjGUn6cBkSrIIaXU/VIl+sjxuKMZYDVOFtoatbuMLaAUlhbjM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707778540; c=relaxed/simple; bh=C1hCLCQn4NR2FTzdKEypLrLJBl7H5vBGsDD9FEekdA0=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=oUws9e67wr5quxP+ZYa8Xq4d/HosOy49qL7JhVKeAjcS4JHApYvjHEagzxa5M4lireubHjqBE/1FyEgl9uXNGFbdWlnGvDQVZjRBo4Cv3UX2zqKwXV6o/dvwVcCJ9PsfeMbogOhCN/TmpQhj+3nw+LBcnkNIGDMgCB9Lu3QmlUI= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-qv1-xf2f.google.com with SMTP id 6a1803df08f44-68c794970d5so35224356d6.0 for ; Mon, 12 Feb 2024 14:55:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=obs-cr.20230601.gappssmtp.com; s=20230601; t=1707778537; x=1708383337; darn=sourceware.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=VCX0+DXyg4VlF+G2lPEzc6Ux8xC/ojADrDy4jIZDafQ=; b=KuXcwjuhhaNhBPXl21KKuCqVN6z1nh2s/U+seqdM/eUuCyBVsUy+tHEsQbzomyZp96 HWdFkHMjzRG445YPNzArBGbNz3fBZdYrnj0d/7VkDUAIxabuDTE+Z62140wCmwTlnxzl dixhVYbB0NG1zOSt+fEZ4ky5l/7WZUBakFAu6GLQnwKYjbm8TvJnC28+XBupBOcMm/Us 6RfLLkNwUVetLQmLyJ3zaN3foE9CCj1z6u/navWRe0y2xuIc7d7Kdvzae+n2HpJ6j4kS UHMZKR5hp1s/daEXVVTauIOhW1cTNQwZqZ4RIv+VG2ehxn6jVHKbj6tmHpuGQAnGdWTk BmcQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707778537; x=1708383337; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=VCX0+DXyg4VlF+G2lPEzc6Ux8xC/ojADrDy4jIZDafQ=; b=tirhM09GkE6F56GCX2DBg1RpGQzHQCJ/k8UOmtAOtJBlFen3ZdWi0Ncm+0hLCp8OIm kTEv1ynr7Ubd74A6EbDILxXmjLdct+eDbfLVK5mpPX+Sy9fq1NxmpzCD+Z4ipCuFQzi+ mkZdiuO0F6TAr2pWBwR7yl+uDcGFsqcwyETKUVtIwXi8BUtdrsmdInFwng7dkuVqwWr+ 7tSo/vUS+3AdXzaOm+ipqvRu+MhqrixrPlcE0tZ0sE4rj02rTpY1V0fZfwLUwLY+EQIZ 7gFzJQFib+0aquyzPHQcurFQeA0XYkCzylkEH5/Eb5rQtXnu3A/Ii0BDUsEdENw1xSMl owmQ== X-Forwarded-Encrypted: i=1; AJvYcCU0ONf6RvjdfVXe10zuf3fcd9vAh/l9hH3t934Vya2TZFZEPDabexy6Vwxwc0Xo/dlw2JgpMDHuh4a3t4u5TcWj54SZ+wRxIg== X-Gm-Message-State: AOJu0Yw09UjE4hlI/p9qrdiyQIdTN0kkhiveDSxUaeBdxFWlGWWHtVEO PPBOpvyBfYxyzKxuxT3Y68ka8/8MiWBU4q+DxJExLdwfK5OMAJe7+EeFFh9Vd3bJahDbDGF8POJ j9HWEanLB2UXe3F20zbEG//wcHmKzC9SMLfbbCg== X-Google-Smtp-Source: AGHT+IF2exeaQiGeZ/bpdsuubdLrBw4dBuuv3uNrZbMHwJHj7du3/2OpKK+0oDhYJcuQjnEdG7Pcghad6zS127Hmq6s= X-Received: by 2002:a0c:db89:0:b0:68c:e7bd:fc49 with SMTP id m9-20020a0cdb89000000b0068ce7bdfc49mr1723777qvk.12.1707778537431; Mon, 12 Feb 2024 14:55:37 -0800 (PST) MIME-Version: 1.0 References: <20240212174209.620310-1-hawkinsw@obs.cr> <87h6idv86g.fsf@oracle.com> In-Reply-To: From: Will Hawkins Date: Mon, 12 Feb 2024 17:55:26 -0500 Message-ID: Subject: Re: [PATCH v2 0/1] Add BPF callx support to objdump and as To: Yonghong Song Cc: "Jose E. Marchesi" , Dave Thaler , binutils@sourceware.org, Eduard Zingerman Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-0.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Mon, Feb 12, 2024 at 5:50=E2=80=AFPM Yonghong Song wrote: > > > On 2/12/24 2:38 PM, Will Hawkins wrote: > > On Mon, Feb 12, 2024 at 5:25=E2=80=AFPM Will Hawkins = wrote: > >> Hello! > >> > >> First, thank you for the response! > >> > >> On Mon, Feb 12, 2024 at 1:39=E2=80=AFPM Jose E. Marchesi > >> wrote: > >>> > >>> Hi Will. > >>> > >>> [Adding Yonghong and Eduard in CC] > >>> > >>>> After additional consideration and discussion with Jose and Dave, > >>>> it seems like we have determined the way that clang, gcc and binutil= s > >>>> need to handle the callx/callr: > >>>> > >>>> 1. callr remains with the register holding the target of the jump st= ored > >>>> in the dst_reg. > >>>> 2. callx is added with the register holding the target of the jump s= tored > >>>> in the imm32. > >>>> 3. We have to remove the pseudoc syntax because it is no longer poss= ible > >>>> to disambiguate between versions of call by simply looking at the > >>>> parameter. > >>> I don't recall reaching any agreement on the above. What is the poin= t > >>> of having both callr and callx? > >> Sorry! I was being slightly loose in terms of agreement -- I was > >> reading into your comments in the email between you, me and Dave from > >> earlier this weekend! > >> > >> The only point in having both callr and callx was to allow the gcc > >> encoding to continue to exist in its current form. I assumed that > >> there was a compelling reason and certainly did not want to do > >> anything to interfere with the great work that you are doing! > >> > >>> The existing callr is generated by GCC in -mxbpf mode. It is an > >>> experimental extension that we use in order to be able to run more of > >>> the GCC testsuite, so it is always possible to change it to use imm32 > >>> instead of dst_reg. > >>> > >>> I wouldn't personally welcome that change and would much prefer if cl= ang > >>> starts using either reg_src or reg_dst, because compromising/reservin= g > >>> endian-dependent 32 whole bits for a register number that only requir= es > >>> 4 bits seems like a waste of insn space that will complicate future I= SA > >>> extensions. > >> I 100% agree that it is less than ideal. However, it seems like the > >> cat is out of the bag. I am adding Dave who is leading the ISA > >> standardization effort. He and I (and others) have discussed this as > >> recently as this morning. I will let him weigh in on whether or not we > >> have the "power" to push back on clang's choice of how to encode the > >> instructions. > >> > >>> In either case, if we all use the same encoding for the indirect call > >>> instruction (I fail to see any reason for not doing so) then point > >>> 3. becomes moot. > >> I agree and I really would like that to be the outcome. However, see > >> above (insert smiley face here!) > >> > > I just reviewed some mailing traffic from another list and it looks > > like the folks at clang/llvm are going to change the way that they > > encode the callx instruction! Great news! > > > > I will make a (simpler) updated patch to binutils once those changes > > are in llvm and we can verify them. > > the llvm patch: > https://github.com/llvm/llvm-project/pull/81546 > Could you help double check encoding is the same as gcc? I would be more than happy to do so! It will be a few hours, but I will absolutely look at it ASAP! Better yet, I will pull that patch, build an LLVM and give it a try to double check. Thank you for working so quickly, Yonghong! Will > > Thanks! > > > > > Thank you again for your response, Jose! > > Will > > > > > >> Thank you for responding! > >> > >> Will > >> > >>>> Tests are added/refactored to meet the above. > >>>> > >>>> I am more than happy to resend as a separate mailing to the list but > >>>> sending first as a reply in order to keep list traffic manageable. > >>>> > >>>> As I said before, I sincerely appreciate all that you are doing for > >>>> the community and how welcoming you have been to a first-time contri= butor. > >>>> > >>>> Sincerely, > >>>> Will > >>>> > >>>> Will Hawkins (1): > >>>> objdump, as: Add callx support for BPF CPU v1 > >>>> > >>>> gas/config/tc-bpf.c | 25 +++++++++++++++= +++- > >>>> gas/testsuite/gas/bpf/bpf.exp | 4 +-- > >>>> .../gas/bpf/{indcall-1.d =3D> callr.d} | 4 +-- > >>>> .../gas/bpf/{indcall-1.s =3D> callr.s} | 2 +- > >>>> gas/testsuite/gas/bpf/indcall-1-pseudoc.d | 23 ---------------= -- > >>>> gas/testsuite/gas/bpf/indcall-1-pseudoc.s | 13 ---------- > >>>> include/opcode/bpf.h | 3 ++- > >>>> opcodes/bpf-dis.c | 6 +++++ > >>>> opcodes/bpf-opc.c | 4 ++- > >>>> sim/bpf/bpf-sim.c | 4 +++ > >>>> 10 files changed, 44 insertions(+), 44 deletions(-) > >>>> rename gas/testsuite/gas/bpf/{indcall-1.d =3D> callr.d} (90%) > >>>> rename gas/testsuite/gas/bpf/{indcall-1.s =3D> callr.s} (90%) > >>>> delete mode 100644 gas/testsuite/gas/bpf/indcall-1-pseudoc.d > >>>> delete mode 100644 gas/testsuite/gas/bpf/indcall-1-pseudoc.s