From: Jan Beulich <jbeulich@suse.com>
To: Binutils <binutils@sourceware.org>
Cc: "H.J. Lu" <hjl.tools@gmail.com>
Subject: [PATCH] x86: corrections to CPU attribute/flags splitting
Date: Fri, 22 Dec 2023 12:26:39 +0100 [thread overview]
Message-ID: <ceaed6a3-dbcf-4554-9b2a-9b8a20388079@suse.com> (raw)
There are a number of issues with 734dfd1cc966 ("x86: pack CPU flags in
opcode table"):
- the condition when two array slots need writing wasn't correct (with
enough new Cpu* added an out of bounds array access would validly have
been complained about by the compiler),
- table generation didn't take into account CpuAttrUnused and CpuUnused
being independent, and hence there not always (not) being an "unused"
bitfield member in both structures,
- cpu_flags_from_attr() wasn't ready for use on big-endian hosts,
- there were two style violations.
---
The big endian part of the change is untested; I hope it's at least
getting close enough, but I have nowhere to test. Of course I'd more
than welcome any suggestions towards removing the endianness dependency,
as long as resulting code remains simple enough (no explicit dealing
with individual .cpu* members) and compiles to a reasonably short
sequence of insns (it's used on a hot path, after all).
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -1689,21 +1689,34 @@ static i386_cpu_flags cpu_flags_from_att
const unsigned int bps = sizeof (a.array[0]) * CHAR_BIT;
i386_cpu_flags f = { .array[0] = 0 };
- switch (ARRAY_SIZE(a.array))
+ switch (ARRAY_SIZE (a.array))
{
case 1:
f.array[CpuAttrEnums / bps]
- |= (a.array[0] >> CpuIsaBits) << (CpuAttrEnums % bps);
- if (CpuAttrEnums % bps > CpuIsaBits)
+#ifndef WORDS_BIGENDIAN
+ |= (a.array[0] >> CpuIsaBits) << (CpuAttrEnums % bps);
+#else
+ |= (a.array[0] << CpuIsaBits) >> (CpuAttrEnums % bps);
+#endif
+ if (CpuMax / bps > CpuAttrEnums / bps)
f.array[CpuAttrEnums / bps + 1]
+#ifndef WORDS_BIGENDIAN
= (a.array[0] >> CpuIsaBits) >> (bps - CpuAttrEnums % bps);
+#else
+ = (a.array[0] << CpuIsaBits) << (bps - CpuAttrEnums % bps);
+#endif
break;
+
default:
abort ();
}
if (a.bitfield.isa)
+#ifndef WORDS_BIGENDIAN
f.array[(a.bitfield.isa - 1) / bps] |= 1u << ((a.bitfield.isa - 1) % bps);
+#else
+ f.array[(a.bitfield.isa - 1) / bps] |= 1u << (~(a.bitfield.isa - 1) % bps);
+#endif
return f;
}
--- a/opcodes/i386-gen.c
+++ b/opcodes/i386-gen.c
@@ -851,7 +851,16 @@ output_cpu_flags (FILE *table, bitfield
active_cpu_flags.array[i / 32] |= 1U << (i % 32);
}
- fprintf (table, "%d } }%s\n", flags[i].value, comma);
+#if defined(CpuAttrUnused) != defined(CpuUnused)
+ if (mode <= 0)
+# ifdef CpuUnused
+ fprintf (table, " } }%s\n", comma);
+# else
+ fprintf (table, "%d, 0 } }%s\n", flags[i].value, comma);
+# endif
+ else
+#endif
+ fprintf (table, "%d } }%s\n", flags[i].value, comma);
}
static void
reply other threads:[~2023-12-22 11:26 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
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=ceaed6a3-dbcf-4554-9b2a-9b8a20388079@suse.com \
--to=jbeulich@suse.com \
--cc=binutils@sourceware.org \
--cc=hjl.tools@gmail.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).