public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: Binutils <binutils@sourceware.org>
Cc: "H.J. Lu" <hjl.tools@gmail.com>
Subject: [PATCH] x86/Intel: SHLD/SHRD have dual meaning
Date: Fri, 19 Apr 2024 11:28:48 +0200	[thread overview]
Message-ID: <117c9c1f-795e-41f9-b582-b8477b42bebd@suse.com> (raw)

Since we uniformly permit D suffixes in Intel mode whenever in AT&T mode
an L suffix may be used, we need to be consistent with this.

Take the easy route, despite that still leading to an anomaly which is
also visible from the new testcase:

	shld	eax, ecx, 1
	shld	eax, ecx, cl

can mean two things with APX: SHL with a D suffix in NDD EVEX encoding,
or the traditional SHLD in legacy encoding.
---
The alternative, more intrusive and more risky (in terms of perceived or
even real regressions) route would be to mark the few insns which permit
suffixes even in Intel syntax, and reject suffix uses when that
indicator isn't set.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -5389,7 +5389,7 @@ static void init_globals (void)
 }
 
 /* Helper for md_assemble() to decide whether to prepare for a possible 2nd
-   parsing pass. Instead of introducing a rarely use new insn attribute this
+   parsing pass. Instead of introducing a rarely used new insn attribute this
    utilizes a common pattern between affected templates. It is deemed
    acceptable that this will lead to unnecessary pass 2 preparations in a
    limited set of cases.  */
@@ -5401,7 +5401,10 @@ static INLINE bool may_need_pass2 (const
 	 : (t->opcode_space == SPACE_0F
 	    && (t->base_opcode | 1) == 0xbf)
 	   || (t->opcode_space == SPACE_BASE
-	       && t->base_opcode == 0x63);
+	       && t->base_opcode == 0x63)
+	   || (intel_syntax /* shld / shrd may mean suffixed shl / shr.  */
+	       && t->opcode_space == SPACE_EVEXMAP4
+	       && (t->base_opcode | 8) == 0x2c);
 }
 
 #if defined (OBJ_MAYBE_ELF) || defined (OBJ_ELF)
--- /dev/null
+++ b/gas/testsuite/gas/i386/intel-suffix.d
@@ -0,0 +1,34 @@
+#objdump: -dw
+#name: Intel syntax w/ suffixes
+
+.*: +file format .*
+
+Disassembly of section \.text:
+0+0 <.*>:
+[ 	]*[a-f0-9]+:	0f a4 c8 01[ 	]+shld   \$0x1,%ecx,%eax
+[ 	]*[a-f0-9]+:	0f a5 c8[ 	]+shld   %cl,%ecx,%eax
+[ 	]*[a-f0-9]+:	d1 e1[ 	]+shl    \$1,%ecx
+[ 	]*[a-f0-9]+:	d3 e1[ 	]+shl    %cl,%ecx
+[ 	]*[a-f0-9]+:	62 f4 7c 18 d1 e1[ 	]+shl    \$1,%ecx,%eax
+[ 	]*[a-f0-9]+:	62 f4 7c 18 d3 e1[ 	]+shl    %cl,%ecx,%eax
+[ 	]*[a-f0-9]+:	d1 e1[ 	]+shl    \$1,%ecx
+[ 	]*[a-f0-9]+:	d3 e1[ 	]+shl    %cl,%ecx
+[ 	]*[a-f0-9]+:	62 f4 7c 18 d1 c1[ 	]+rol    \$1,%ecx,%eax
+[ 	]*[a-f0-9]+:	62 f4 7c 18 d3 c1[ 	]+rol    %cl,%ecx,%eax
+[ 	]*[a-f0-9]+:	d1 c1[ 	]+rol    \$1,%ecx
+[ 	]*[a-f0-9]+:	d3 c1[ 	]+rol    %cl,%ecx
+
+0+[0-9a-f]+ <.*>:
+[ 	]*[a-f0-9]+:	0f ac c8 01[ 	]+shrd   \$0x1,%ecx,%eax
+[ 	]*[a-f0-9]+:	0f ad c8[ 	]+shrd   %cl,%ecx,%eax
+[ 	]*[a-f0-9]+:	d1 e9[ 	]+shr    \$1,%ecx
+[ 	]*[a-f0-9]+:	d3 e9[ 	]+shr    %cl,%ecx
+[ 	]*[a-f0-9]+:	62 f4 7c 18 d1 f9[ 	]+sar    \$1,%ecx,%eax
+[ 	]*[a-f0-9]+:	62 f4 7c 18 d3 f9[ 	]+sar    %cl,%ecx,%eax
+[ 	]*[a-f0-9]+:	d1 f9[ 	]+sar    \$1,%ecx
+[ 	]*[a-f0-9]+:	d3 f9[ 	]+sar    %cl,%ecx
+[ 	]*[a-f0-9]+:	62 f4 7c 18 d1 c9[ 	]+ror    \$1,%ecx,%eax
+[ 	]*[a-f0-9]+:	62 f4 7c 18 d3 c9[ 	]+ror    %cl,%ecx,%eax
+[ 	]*[a-f0-9]+:	d1 c9[ 	]+ror    \$1,%ecx
+[ 	]*[a-f0-9]+:	d3 c9[ 	]+ror    %cl,%ecx
+#pass
--- /dev/null
+++ b/gas/testsuite/gas/i386/intel-suffix.s
@@ -0,0 +1,39 @@
+	.intel_syntax noprefix
+	.text
+left:
+	shld	eax, ecx, 1
+	shld	eax, ecx, cl
+
+	shld	ecx, 1
+	shld	ecx, cl
+
+	sald	eax, ecx, 1
+	sald	eax, ecx, cl
+
+	sald	ecx, 1
+	sald	ecx, cl
+
+	rold	eax, ecx, 1
+	rold	eax, ecx, cl
+
+	rold	ecx, 1
+	rold	ecx, cl
+
+right:
+	shrd	eax, ecx, 1
+	shrd	eax, ecx, cl
+
+	shrd	ecx, 1
+	shrd	ecx, cl
+
+	sard	eax, ecx, 1
+	sard	eax, ecx, cl
+
+	sard	ecx, 1
+	sard	ecx, cl
+
+	rord	eax, ecx, 1
+	rord	eax, ecx, cl
+
+	rord	ecx, 1
+	rord	ecx, cl
--- a/gas/testsuite/gas/i386/x86-64.exp
+++ b/gas/testsuite/gas/i386/x86-64.exp
@@ -160,6 +160,7 @@ run_dump_test "x86-64-disp-intel"
 run_list_test "disp-imm-64"
 run_dump_test "intel-movs64"
 run_dump_test "intel-cmps64"
+run_dump_test "intel-suffix"
 run_dump_test "x86-64-disp32"
 run_dump_test "rexw"
 run_list_test "x86-64-specific-reg"

             reply	other threads:[~2024-04-19  9:28 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-19  9:28 Jan Beulich [this message]
2024-04-22  4:09 ` Hongtao Liu
2024-04-22  6:51   ` Jan Beulich
2024-04-22  7:43     ` Cui, Lili
2024-04-22  7:47       ` Hongtao Liu
2024-04-22  8:42         ` Jan Beulich
2024-04-22 11:36           ` Hongtao Liu
2024-04-22  8:45       ` Jan Beulich

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=117c9c1f-795e-41f9-b582-b8477b42bebd@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).