public inbox for cgen@sourceware.org
 help / color / mirror / Atom feed
From: Alan Modra <amodra@gmail.com>
To: Guillermo Martinez <guillermo.e.martinez@oracle.com>,
	"Jose E. Marchesi" <jose.marchesi@oracle.com>
Cc: cgen@sourceware.org, binutils@sourceware.org
Subject: Re: [PATCH v2] cgen: Compute correct mask and values when offset in define-ifield is not 0.
Date: Fri, 26 Nov 2021 09:32:11 +1030	[thread overview]
Message-ID: <YaAV83hDtXjBYEye@squeak.grove.modra.org> (raw)
In-Reply-To: <1807213.tdWV9SEqCh@sali>

On Wed, Sep 15, 2021 at 06:04:44PM +0000, Guillermo Martinez wrote:
> On Wednesday, September 15, 2021 11:49:12 AM CDT Jose E. Marchesi wrote:
> > I just installed this version of the patch on your behalf.
> > Thanks!
> > 
> > > If an instruction field is defined in a long form, assigning
> > > an offset different to 0 the mask and constant values are not
> > > computed appropriately:
[snip]

It's been a while since I refreshed the binutils opcodes files that
are cgen generated.  On doing so it appears this patch is responsible
for build errors when using mainline gcc.  First example:

/home/alan/src/binutils-gdb/opcodes/bpf-opc.c:57:11: error: conversion from ‘long unsigned int’ to ‘unsigned int’ changes value from ‘18446744073709486335’ to ‘4294902015’ [-Werror=overflow]
   57 |   64, 64, 0xffffffffffff00ff, { { F (F_IMM32) }, { F (F_OFFSET16) }, { F (F_SRCLE) }, { F (F_OP_CODE) }, { F (F_DSTLE) }, { F (F_OP_SRC) }, { F (F_OP_CLASS) }, { 0 } }
      |           ^~~~~~~~~~~~~~~~~~

I tested an obvious workaround,

diff --git a/include/opcode/cgen.h b/include/opcode/cgen.h
index 8b7d2a4b547..0e9571d19b0 100644
--- a/include/opcode/cgen.h
+++ b/include/opcode/cgen.h
@@ -914,7 +914,7 @@ typedef struct
      Each insn's value is stored with the insn.
      The first step in recognizing an insn for disassembly is
      (opcode & mask) == value.  */
-  CGEN_INSN_INT mask;
+  uint64_t mask;
 #define CGEN_IFMT_MASK(ifmt) ((ifmt)->mask)
 
   /* Instruction fields.
diff --git a/opcodes/cgen-dis.c b/opcodes/cgen-dis.c
index 1a5d1ae8459..37ee5a23564 100644
--- a/opcodes/cgen-dis.c
+++ b/opcodes/cgen-dis.c
@@ -39,7 +39,7 @@ static void		 add_insn_to_hash_chain (CGEN_INSN_LIST *,
 static int
 count_decodable_bits (const CGEN_INSN *insn)
 {
-  unsigned mask = CGEN_INSN_BASE_MASK (insn);
+  uint64_t mask = CGEN_INSN_BASE_MASK (insn);
 #if GCC_VERSION >= 3004
   return __builtin_popcount (mask);
 #else

Quite possibly the above isn't a complete fix, I just threw it
together to see what happens.  Regressions:
+FAIL: eBPF CALL instruction
+FAIL: eBPF CALL instruction, big endian
+FAIL: CALL with disp32 reloc
+FAIL: CALL with disp32 reloc and addend
+FAIL: CALL check unsigned underflow

gas/testsuite/gas.log for the first one shows:
regexp_diff match failure
regexp "^  20:  85 10 00 00 00 00 00 00         call 0$"
line   "  20:   85 10 00 00 00 00 00 00         *unknown*"
regexp_diff match failure
regexp "^  28:  85 10 00 00 ff ff ff ff         call -1$"
line   "  28:   85 10 00 00 ff ff ff ff         *unknown*"
regexp_diff match failure
regexp "^  30:  85 10 00 00 fe ff ff ff         call -2$"
line   "  30:   85 10 00 00 fe ff ff ff         *unknown*"
regexp_diff match failure
regexp "^  38:  85 10 00 00 fd ff ff ff         call -3$"
line   "  38:   85 10 00 00 fd ff ff ff         *unknown*"
FAIL: eBPF CALL instruction

At this point I don't intend to look any further into the problem.
The opcodes/ refresh can wait.

-- 
Alan Modra
Australia Development Lab, IBM

  reply	other threads:[~2021-11-25 23:02 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-20  2:32 [PATCH CGEN v1] " Guillermo E. Martinez
2021-09-02 13:22 ` Guillermo Martinez
2021-09-10  1:23   ` Frank Ch. Eigler
2021-09-10  2:29     ` Jose E. Marchesi
2021-09-10  2:36       ` Jose E. Marchesi
2021-09-14 21:34 ` Jose E. Marchesi
2021-09-15 16:32 ` [PATCH v2] " Guillermo E. Martinez
2021-09-15 16:49   ` Jose E. Marchesi
2021-09-15 18:04     ` Guillermo Martinez
2021-11-25 23:02       ` Alan Modra [this message]
2021-11-25 23:28         ` Guillermo Martinez
2021-11-26  0:46           ` Alan Modra

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=YaAV83hDtXjBYEye@squeak.grove.modra.org \
    --to=amodra@gmail.com \
    --cc=binutils@sourceware.org \
    --cc=cgen@sourceware.org \
    --cc=guillermo.e.martinez@oracle.com \
    --cc=jose.marchesi@oracle.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).