diff mbox series

[v2] x86: Support ZHAOXIN GMI instructions

Message ID cad3cfeb-6589-4495-9e10-4e57989319fb@zhaoxin.com
State New
Headers
Series [v2] x86: Support ZHAOXIN GMI instructions |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Mayshao-oc Oct. 15, 2024, 1:19 a.m. UTC 
  Hi all:

        Thanks for the very insightful comments of Jan hand haochen, I 
refine the patch.
        Test ok, ok for trunk?

BR
Mayshao

Comments

Jiang, Haochen Oct. 15, 2024, 3:02 a.m. UTC | #1 
> From: mayshao-oc <mayshao-oc@zhaoxin.com>
> Sent: Tuesday, October 15, 2024 9:19 AM
> 
> Hi all:
> 
>         Thanks for the very insightful comments of Jan hand haochen, I refine the
> patch.
>         Test ok, ok for trunk?

@@ -1050,6 +1051,9 @@ enum
   PREFIX_0F7D,
   PREFIX_0F7E,
   PREFIX_0F7F,
+  PREFIX_0FA6_REG_0_MOD_3,
+  PREFIX_0FA6_REG_5_MOD_3,
+  PREFIX_0FA7_REG_6_MOD_3,
   PREFIX_0FAE_REG_0_MOD_3,
   PREFIX_0FAE_REG_1_MOD_3,
   PREFIX_0FAE_REG_2_MOD_3,

I suppose the table entries here do not need _MOD_3 suffix since it did not use
mod_table[].

+  /* PREFIX_0FA6_REG_5_MOD_3 */
+  {
+    { Bad_Opcode },
+    { "sm3",	{ Skip_MODRM }, 0 },
+    { Bad_Opcode },
+    { Bad_Opcode },
+  },
+
+  /* PREFIX_0FA7_REG_6_MOD_3 */
+  {
+    { Bad_Opcode },
+    { "sm4",	{ Skip_MODRM }, 0 },
+    { Bad_Opcode },
+    { Bad_Opcode },
+  },
+

The Bad_Opcode could be omitted if they are at the end of
a table.

@@ -13086,6 +13118,21 @@ OP_0f07 (instr_info *ins, int bytemode, int sizeflag)
   return OP_E (ins, bytemode, sizeflag);
 }
 
+/* montmul instruction need display repz and skip modrm */
+
+static bool
+MONTMUL_Fixup (instr_info *ins, int bytemode, int sizeflag)
+{
+  (void) bytemode;
+  (void) sizeflag;

What are these two lines for? And you will need MODRM_CHECK;
and modrm.mod invalid check. You could take OP_Skip_MODRM
as reference.

+  /* The 0xf3 prefix should be displayed as "repz" for montmul. */
+  if (ins->prefixes & PREFIX_REPZ)
+    ins->all_prefixes[ins->last_repz_prefix] = 0xf3;
+  ins->codep++;
+  ins->has_skipped_modrm = true;
+  return true;
+}
+

I still don't know why original montmul will display repz prefix although
the testcase does show that. I checked the PadLock doc but got no clue
and even more confused. Could you explain why?

Also, in gas/NEWS, could you put the entry at the very beginning?

Thx,
Haochen

> 
> BR
> Mayshao
Jan Beulich Oct. 15, 2024, 5:53 a.m. UTC | #2 
On 15.10.2024 05:02, Jiang, Haochen wrote:
>> From: mayshao-oc <mayshao-oc@zhaoxin.com>
>> Sent: Tuesday, October 15, 2024 9:19 AM
>>
> @@ -13086,6 +13118,21 @@ OP_0f07 (instr_info *ins, int bytemode, int sizeflag)
>    return OP_E (ins, bytemode, sizeflag);
>  }
>  
> +/* montmul instruction need display repz and skip modrm */
> +
> +static bool
> +MONTMUL_Fixup (instr_info *ins, int bytemode, int sizeflag)
> +{
> +  (void) bytemode;
> +  (void) sizeflag;
> 
> What are these two lines for?

ATTRIBUTE_UNUSED wants using instead.

> And you will need MODRM_CHECK;
> and modrm.mod invalid check. You could take OP_Skip_MODRM
> as reference.

I think OP_Skip_MODRM() simply wants invoking from here. That'll
deal with ...

> +  /* The 0xf3 prefix should be displayed as "repz" for montmul. */
> +  if (ins->prefixes & PREFIX_REPZ)
> +    ins->all_prefixes[ins->last_repz_prefix] = 0xf3;
> +  ins->codep++;
> +  ins->has_skipped_modrm = true;
> +  return true;
> +}

... part of the omissions: Right now both modrm.mod and modrm.rm
aren't being checked here.

> Also, in gas/NEWS, could you put the entry at the very beginning?

I don't think the placement there matters much, as long as it's in the
correct section.

Jan
Mayshao-oc Oct. 15, 2024, 6:51 a.m. UTC | #3 
Hi Haochen:

On 10/15/24 11:02, Jiang, Haochen wrote:
> 
> 
> 
>> From: mayshao-oc <mayshao-oc@zhaoxin.com>
>> Sent: Tuesday, October 15, 2024 9:19 AM
>>
>> Hi all:
>>
>>          Thanks for the very insightful comments of Jan hand haochen, I refine the
>> patch.
>>          Test ok, ok for trunk?
> 
> @@ -1050,6 +1051,9 @@ enum
>     PREFIX_0F7D,
>     PREFIX_0F7E,
>     PREFIX_0F7F,
> +  PREFIX_0FA6_REG_0_MOD_3,
> +  PREFIX_0FA6_REG_5_MOD_3,
> +  PREFIX_0FA7_REG_6_MOD_3,
>     PREFIX_0FAE_REG_0_MOD_3,
>     PREFIX_0FAE_REG_1_MOD_3,
>     PREFIX_0FAE_REG_2_MOD_3,
> 
> I suppose the table entries here do not need _MOD_3 suffix since it did not use
> mod_table[].
I suppose use mod_table[] is not a prerequisite to define a enum 
constant, I trade readability for simplicity. I define it like this, 
because it has mod value, not since it use mod_table. And there are 
examples to use MOD in it's name, but don't use mod_table, 
PREFIX_0FAE_REG_5_MOD_3.Correct me if I am wrong.
 >
> 
> +  /* PREFIX_0FA6_REG_5_MOD_3 */
> +  {
> +    { Bad_Opcode },
> +    { "sm3",   { Skip_MODRM }, 0 },
> +    { Bad_Opcode },
> +    { Bad_Opcode },
> +  },
> +
> +  /* PREFIX_0FA7_REG_6_MOD_3 */
> +  {
> +    { Bad_Opcode },
> +    { "sm4",   { Skip_MODRM }, 0 },
> +    { Bad_Opcode },
> +    { Bad_Opcode },
> +  },
> +
> 
> The Bad_Opcode could be omitted if they are at the end of
> a table.
As above, I think its a code style issue, and there are some Bad_Opcode 
at the end of a table, like in PREFIX_0F01_REG_1_RM_4, 
PREFIX_0F01_REG_1_RM_5. And I think Bad_Opcode is a placeholder, and 
there is no detrimental effect to define some Bad_Opcode at the end of a 
table. Correct me if I am wrong.
> 
> @@ -13086,6 +13118,21 @@ OP_0f07 (instr_info *ins, int bytemode, int sizeflag)
>     return OP_E (ins, bytemode, sizeflag);
>   }
> 
> +/* montmul instruction need display repz and skip modrm */
> +
> +static bool
> +MONTMUL_Fixup (instr_info *ins, int bytemode, int sizeflag)
> +{
> +  (void) bytemode;
> +  (void) sizeflag;
> 
> What are these two lines for? And you will need MODRM_CHECK;
> and modrm.mod invalid check. You could take OP_Skip_MODRM
> as reference.
> 
These two lines are added to avoid the "unused parameter error",
ATTIBUTE_UNUSED is a elegant way to do this.
I don't figure out why I need MODRM_CHECK, MODRM_CHECK could prevent 
what risk.
> +  /* The 0xf3 prefix should be displayed as "repz" for montmul. */
> +  if (ins->prefixes & PREFIX_REPZ)
> +    ins->all_prefixes[ins->last_repz_prefix] = 0xf3;
> +  ins->codep++;
> +  ins->has_skipped_modrm = true;
> +  return true;
> +}
> +
 >
> I still don't know why original montmul will display repz prefix although
> the testcase does show that. I checked the PadLock doc but got no clue
> and even more confused. Could you explain why?
> 
I don't know why, I just do this for the backward compatibility. From my 
own perspective, there is no need to display repz, because there is no
rep related meaning for the montmul instruction in the padlock doc. But 
I don't want to make the legacy padlock testsuite fail.
> Also, in gas/NEWS, could you put the entry at the very beginning?
> 
> Thx,
> Haochen
> 
>>
>> BR
>> Mayshao

BR
Mayshao
Jan Beulich Oct. 15, 2024, 11:18 a.m. UTC | #4 
On 15.10.2024 08:51, mayshao-oc wrote:
> On 10/15/24 11:02, Jiang, Haochen wrote:
>>> From: mayshao-oc <mayshao-oc@zhaoxin.com>
>>> Sent: Tuesday, October 15, 2024 9:19 AM
>>>
>> @@ -1050,6 +1051,9 @@ enum
>>     PREFIX_0F7D,
>>     PREFIX_0F7E,
>>     PREFIX_0F7F,
>> +  PREFIX_0FA6_REG_0_MOD_3,
>> +  PREFIX_0FA6_REG_5_MOD_3,
>> +  PREFIX_0FA7_REG_6_MOD_3,
>>     PREFIX_0FAE_REG_0_MOD_3,
>>     PREFIX_0FAE_REG_1_MOD_3,
>>     PREFIX_0FAE_REG_2_MOD_3,
>>
>> I suppose the table entries here do not need _MOD_3 suffix since it did not use
>> mod_table[].
> I suppose use mod_table[] is not a prerequisite to define a enum 
> constant, I trade readability for simplicity. I define it like this, 
> because it has mod value, not since it use mod_table. And there are 
> examples to use MOD in it's name, but don't use mod_table, 
> PREFIX_0FAE_REG_5_MOD_3.Correct me if I am wrong.

There may be misnamed enumerators, but we shouldn't repeat prior mistakes
(and I've tried to clean up some of that earlier on). The naming here is
strictly by what earlier decode steps were taken, and in what order.

>> +  /* PREFIX_0FA6_REG_5_MOD_3 */
>> +  {
>> +    { Bad_Opcode },
>> +    { "sm3",   { Skip_MODRM }, 0 },
>> +    { Bad_Opcode },
>> +    { Bad_Opcode },
>> +  },
>> +
>> +  /* PREFIX_0FA7_REG_6_MOD_3 */
>> +  {
>> +    { Bad_Opcode },
>> +    { "sm4",   { Skip_MODRM }, 0 },
>> +    { Bad_Opcode },
>> +    { Bad_Opcode },
>> +  },
>> +
>>
>> The Bad_Opcode could be omitted if they are at the end of
>> a table.
> As above, I think its a code style issue, and there are some Bad_Opcode 
> at the end of a table, like in PREFIX_0F01_REG_1_RM_4, 
> PREFIX_0F01_REG_1_RM_5. And I think Bad_Opcode is a placeholder, and 
> there is no detrimental effect to define some Bad_Opcode at the end of a 
> table. Correct me if I am wrong.

While indeed largely a style issue, the file is too large already anyway,
so we prefer to omit from tables whatever can easily be omitted.

Jan
Mayshao-oc Oct. 15, 2024, 11:44 a.m. UTC | #5 
Hi Jan:

       Thanks for your patience, and I refine the patch to name the 
enumerators properly.

BR
Mayshao

On 10/15/24 19:18, Jan Beulich wrote:
> 
> 
> 
> On 15.10.2024 08:51, mayshao-oc wrote:
>> On 10/15/24 11:02, Jiang, Haochen wrote:
>>>> From: mayshao-oc <mayshao-oc@zhaoxin.com>
>>>> Sent: Tuesday, October 15, 2024 9:19 AM
>>>>
>>> @@ -1050,6 +1051,9 @@ enum
>>>      PREFIX_0F7D,
>>>      PREFIX_0F7E,
>>>      PREFIX_0F7F,
>>> +  PREFIX_0FA6_REG_0_MOD_3,
>>> +  PREFIX_0FA6_REG_5_MOD_3,
>>> +  PREFIX_0FA7_REG_6_MOD_3,
>>>      PREFIX_0FAE_REG_0_MOD_3,
>>>      PREFIX_0FAE_REG_1_MOD_3,
>>>      PREFIX_0FAE_REG_2_MOD_3,
>>>
>>> I suppose the table entries here do not need _MOD_3 suffix since it did not use
>>> mod_table[].
>> I suppose use mod_table[] is not a prerequisite to define a enum
>> constant, I trade readability for simplicity. I define it like this,
>> because it has mod value, not since it use mod_table. And there are
>> examples to use MOD in it's name, but don't use mod_table,
>> PREFIX_0FAE_REG_5_MOD_3.Correct me if I am wrong.
> 
> There may be misnamed enumerators, but we shouldn't repeat prior mistakes
> (and I've tried to clean up some of that earlier on). The naming here is
> strictly by what earlier decode steps were taken, and in what order.
> 
>>> +  /* PREFIX_0FA6_REG_5_MOD_3 */
>>> +  {
>>> +    { Bad_Opcode },
>>> +    { "sm3",   { Skip_MODRM }, 0 },
>>> +    { Bad_Opcode },
>>> +    { Bad_Opcode },
>>> +  },
>>> +
>>> +  /* PREFIX_0FA7_REG_6_MOD_3 */
>>> +  {
>>> +    { Bad_Opcode },
>>> +    { "sm4",   { Skip_MODRM }, 0 },
>>> +    { Bad_Opcode },
>>> +    { Bad_Opcode },
>>> +  },
>>> +
>>>
>>> The Bad_Opcode could be omitted if they are at the end of
>>> a table.
>> As above, I think its a code style issue, and there are some Bad_Opcode
>> at the end of a table, like in PREFIX_0F01_REG_1_RM_4,
>> PREFIX_0F01_REG_1_RM_5. And I think Bad_Opcode is a placeholder, and
>> there is no detrimental effect to define some Bad_Opcode at the end of a
>> table. Correct me if I am wrong.
> 
> While indeed largely a style issue, the file is too large already anyway,
> so we prefer to omit from tables whatever can easily be omitted.
> 
> Jan
Jiang, Haochen Oct. 16, 2024, 2:08 a.m. UTC | #6 
> From: mayshao-oc <mayshao-oc@zhaoxin.com>
> Sent: Tuesday, October 15, 2024 2:52 PM
> 
>  >
> > I still don't know why original montmul will display repz prefix although
> > the testcase does show that. I checked the PadLock doc but got no clue
> > and even more confused. Could you explain why?
> >
> I don't know why, I just do this for the backward compatibility. From my
> own perspective, there is no need to display repz, because there is no
> rep related meaning for the montmul instruction in the padlock doc. But
> I don't want to make the legacy padlock testsuite fail.

Ok I see then.

I suppose disassembler display should not break anything and the current is
somehow buggy and testcase might need a change. But I am ok with keeping
them since the original implementation is 20 years ago and there might be some
reasons doing so. Not changing them is a safe choice.

Thx,
Haochen
Jan Beulich Oct. 16, 2024, 6:47 a.m. UTC | #7 
On 15.10.2024 13:44, mayshao-oc wrote:
>        Thanks for your patience, and I refine the patch to name the 
> enumerators properly.

Just one remaining issue: In your switching to the use of OP_Skip_MODRM()
you lost (or forgot to add?) the ins->modrm.rm != 0 check that the mis-
named OP_0f07() has, and that also applies to these new insns (aiui).

Jan
Mayshao-oc Oct. 16, 2024, 7:48 a.m. UTC | #8 
On 10/16/24 14:47, Jan Beulich wrote:
> 
> 
> 
> On 15.10.2024 13:44, mayshao-oc wrote:
>>         Thanks for your patience, and I refine the patch to name the
>> enumerators properly.
> 
> Just one remaining issue: In your switching to the use of OP_Skip_MODRM()
> you lost (or forgot to add?) the ins->modrm.rm != 0 check that the mis-
> named OP_0f07() has, and that also applies to these new insns (aiui) >
> Jan
>


Hi Jan:

       Thanks for your review. I fix the issue, ok for trunk?

BR
Mayshao
Jan Beulich Oct. 16, 2024, 9:22 a.m. UTC | #9 
On 16.10.2024 09:48, mayshao-oc wrote:
> On 10/16/24 14:47, Jan Beulich wrote:
>> On 15.10.2024 13:44, mayshao-oc wrote:
>>>         Thanks for your patience, and I refine the patch to name the
>>> enumerators properly.
>>
>> Just one remaining issue: In your switching to the use of OP_Skip_MODRM()
>> you lost (or forgot to add?) the ins->modrm.rm != 0 check that the mis-
>> named OP_0f07() has, and that also applies to these new insns (aiui) >
> 
>        Thanks for your review. I fix the issue, ok for trunk?

Mind me asking to move the new conditional ahead of the one that was
already there in (plain) v3? Okay with that adjustment, but before
committing please give Haochen a day or two for possible further
remarks / requests.

Jan
Mayshao-oc Oct. 17, 2024, 2:17 a.m. UTC | #10 
Hi Jan:
        I move the conditional check ahead, and update the patch. Thanks 
for your comments.

BR
Mayshao

On 10/16/24 17:22, Jan Beulich wrote:
> 
> 
> 
> On 16.10.2024 09:48, mayshao-oc wrote:
>> On 10/16/24 14:47, Jan Beulich wrote:
>>> On 15.10.2024 13:44, mayshao-oc wrote:
>>>>          Thanks for your patience, and I refine the patch to name the
>>>> enumerators properly.
>>>
>>> Just one remaining issue: In your switching to the use of OP_Skip_MODRM()
>>> you lost (or forgot to add?) the ins->modrm.rm != 0 check that the mis-
>>> named OP_0f07() has, and that also applies to these new insns (aiui) >
>>
>>         Thanks for your review. I fix the issue, ok for trunk?
> 
> Mind me asking to move the new conditional ahead of the one that was
> already there in (plain) v3? Okay with that adjustment, but before
> committing please give Haochen a day or two for possible further
> remarks / requests.
> 
> Jan
Jan Beulich Oct. 17, 2024, 5:35 a.m. UTC | #11 
On 17.10.2024 04:17, mayshao-oc wrote:
>        I move the conditional check ahead, and update the patch. Thanks for your comments.

Great, feel free to put in then.

Jan
Mayshao-oc Oct. 18, 2024, 12:15 a.m. UTC | #12 
Hi Jan:
       I thinks this patch submission process is a great experience for 
me, so thanks you again. Could you please commit the patch on my behalf?

BR
Mayshao

On 10/17/24 13:35, Jan Beulich wrote:
> 
> 
> 
> On 17.10.2024 04:17, mayshao-oc wrote:
>>         I move the conditional check ahead, and update the patch. Thanks for your comments.
> 
> Great, feel free to put in then.
> 
> Jan
Jiang, Haochen Oct. 18, 2024, 7:38 a.m. UTC | #13 
> From: mayshao-oc <mayshao-oc@zhaoxin.com>
> Sent: Friday, October 18, 2024 8:16 AM
> 
> Hi Jan:
>        I thinks this patch submission process is a great experience for
> me, so thanks you again. Could you please commit the patch on my behalf?
> 

I suppose Jan is quite busy today, I can do that for you to make it happen this
week.

BRs,
Haochen
Jiang, Haochen Oct. 18, 2024, 7:47 a.m. UTC | #14 
> From: Jiang, Haochen
> Sent: Friday, October 18, 2024 3:39 PM
> 
> > From: mayshao-oc <mayshao-oc@zhaoxin.com>
> > Sent: Friday, October 18, 2024 8:16 AM
> >
> > Hi Jan:
> >        I thinks this patch submission process is a great experience
> > for me, so thanks you again. Could you please commit the patch on my
> behalf?
> >
> 
> I suppose Jan is quite busy today, I can do that for you to make it happen this
> week.

Committed as https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=8f0d880434b84b222f87c4ee8f9d6d31d330668d

Thx,
Haochen

> 
> BRs,
> Haochen
Jiang, Haochen Oct. 18, 2024, 7:59 a.m. UTC | #15 
> Committed as https://sourceware.org/git/?p=binutils-
> gdb.git;a=commit;h=8f0d880434b84b222f87c4ee8f9d6d31d330668d

Oops, I forgot the regen file is not in the patch.

Committed a patch as obvious to regenerate them.
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=b2841da4f20b83b22fb46d82a86fe53ba7448469

Thx,
Haochen
Mayshao-oc Oct. 18, 2024, 8:08 a.m. UTC | #16 
Hi Haochen:

            Thank you. I think all of you are models of the guideline of 
<<gnu binutils code of conduct>>. I also want to apply the guideline 
when I communicate to others. I think the community is fun, and hope for 
more communication with all of you in the long term.

BR
Mayshao

On 10/18/24 15:38, Jiang, Haochen wrote:
> 
> 
> 
>> From: mayshao-oc <mayshao-oc@zhaoxin.com>
>> Sent: Friday, October 18, 2024 8:16 AM
>>
>> Hi Jan:
>>         I thinks this patch submission process is a great experience for
>> me, so thanks you again. Could you please commit the patch on my behalf?
>>
> 
> I suppose Jan is quite busy today, I can do that for you to make it happen this
> week.
> 
> BRs,
> Haochen
Jan Beulich Oct. 18, 2024, 11:19 a.m. UTC | #17 
On 18.10.2024 09:47, Jiang, Haochen wrote:
>> From: Jiang, Haochen
>> Sent: Friday, October 18, 2024 3:39 PM
>>
>>> From: mayshao-oc <mayshao-oc@zhaoxin.com>
>>> Sent: Friday, October 18, 2024 8:16 AM
>>>
>>> Hi Jan:
>>>        I thinks this patch submission process is a great experience
>>> for me, so thanks you again. Could you please commit the patch on my
>> behalf?
>>>
>>
>> I suppose Jan is quite busy today, I can do that for you to make it happen this
>> week.
> 
> Committed as https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=8f0d880434b84b222f87c4ee8f9d6d31d330668d

That wasn't the best idea - I had some small tweaks to make, and I was just
about to get to committing. I was asked to do the commit, and the request
wasn't even a day old. Please be a little more patient in the future.

Jan
Jiang, Haochen Oct. 21, 2024, 5:45 a.m. UTC | #18 
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Friday, October 18, 2024 7:20 PM
> On 18.10.2024 09:47, Jiang, Haochen wrote:
> 
> That wasn't the best idea - I had some small tweaks to make, and I was just
> about to get to committing. I was asked to do the commit, and the request
> wasn't even a day old. Please be a little more patient in the future.

I am sorry for that. Typically I would wait for that. But last week's scenario is
that it is near the off-hour for Zhaoxin guys. I committed that patch to make
them not worried too much during the weekend and kept getting online to
see if the patch is committed. My apologies.

Thx,
Haochen

> 
> Jan
diff mbox series

Patch

From 5f51e0f69c6ee4b9f5c5381a03390ce6de5f37f4 Mon Sep 17 00:00:00 2001
From: MayShao-oc <MayShao-oc@zhaoxin.com>
Date: Tue, 15 Oct 2024 08:53:54 +0800
Subject: [PATCH v2] x86: Support ZHAOXIN GMI

Hi all:
      I refine the ZHAOXIN GMI patch, according to the very insightful
comments of Jan and Haochen.Thanks to Jan and Haochen.
      Test ok, ok for trunk?

BR
Mayshao

gas/ChangeLog:

	* NEWS: Support x86 ZHAOXIN GMI instructions.
	* config/tc-i386.c: Add gmi.
	* doc/c-i386.texi: Document gmi.
	* testsuite/gas/i386/i386.exp: Add gmi test.
	* testsuite/gas/i386/gmi.d: Ditto.
	* testsuite/gas/i386/gmi.s: Ditto.

opcodes/ChangeLog:

	* i386-dis.c (MONTMUL_Fixup): New fixup function to disassemble
          montmul.
	* i386-gen.c: Add gmi.
	* i386-opc.h (CpuGMI): New.
	* i386-opc.tbl: Add Zhaoxin GMI instructions.
	* i386-tbl.h: Regenerated.
	* i386-mnem.h: Ditto.
	* i386-init.h: Ditto.

---
 gas/NEWS                        |  2 ++
 gas/config/tc-i386.c            |  1 +
 gas/doc/c-i386.texi             |  6 ++--
 gas/testsuite/gas/i386/gmi.d    | 12 ++++++++
 gas/testsuite/gas/i386/gmi.s    |  8 ++++++
 gas/testsuite/gas/i386/i386.exp |  1 +
 opcodes/i386-dis.c              | 49 ++++++++++++++++++++++++++++++++-
 opcodes/i386-gen.c              |  1 +
 opcodes/i386-opc.h              |  3 ++
 opcodes/i386-opc.tbl            |  5 ++++
 10 files changed, 85 insertions(+), 3 deletions(-)
 create mode 100644 gas/testsuite/gas/i386/gmi.d
 create mode 100644 gas/testsuite/gas/i386/gmi.s

diff --git a/gas/NEWS b/gas/NEWS
index d64330143b0..053b2bcac87 100644
--- a/gas/NEWS
+++ b/gas/NEWS
@@ -5,6 +5,8 @@ 
 * Add support for RISC-V Zcmp (cm.mva01s, cm.mvsa01), Smrnmi and CORE-V
   (xcvbitmanip, xcvsimd) extensions with version 1.0.
 
+* Add support for the x86 Zhaoxin GMI instructions.
+
 Changes in 2.43:
 
 * Add support for LoongArch .option for fine-grained control of assembly
diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index cdefde03717..735154ee721 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -1219,6 +1219,7 @@  static const arch_entry cpu_arch[] =
   SUBARCH (user_msr, USER_MSR, USER_MSR, false),
   SUBARCH (apx_f, APX_F, APX_F, false),
   VECARCH (avx10.2, AVX10_2, ANY_AVX10_2, set),
+  SUBARCH (gmi, GMI, GMI, false),
 };
 
 #undef SUBARCH
diff --git a/gas/doc/c-i386.texi b/gas/doc/c-i386.texi
index 9667061752d..952ddbc7377 100644
--- a/gas/doc/c-i386.texi
+++ b/gas/doc/c-i386.texi
@@ -274,7 +274,8 @@  accept various extension mnemonics.  For example,
 @code{snp},
 @code{invlpgb},
 @code{tlbsync},
-@code{svme} and
+@code{svme},
+@code{gmi} and
 @code{padlock}.
 Note that these extension mnemonics can be prefixed with @code{no} to revoke
 the respective (and any dependent) functionality.  Note further that the
@@ -1705,7 +1706,8 @@  supported on the CPU specified.  The choices for @var{cpu_type} are:
 @item @samp{.lwp} @tab @samp{.fma4} @tab @samp{.xop} @tab @samp{.cx16}
 @item @samp{.padlock} @tab @samp{.clzero} @tab @samp{.mwaitx} @tab @samp{.rdpru}
 @item @samp{.mcommit} @tab @samp{.sev_es} @tab @samp{.snp} @tab @samp{.invlpgb}
-@item @samp{.tlbsync} @tab @samp{.apx_f}
+@item @samp{.tlbsync} @tab @samp{.apx_f} @tab @samp{.gmi}
+
 @end multitable
 
 Apart from the warning, there are only two other effects on
diff --git a/gas/testsuite/gas/i386/gmi.d b/gas/testsuite/gas/i386/gmi.d
new file mode 100644
index 00000000000..063da8e120a
--- /dev/null
+++ b/gas/testsuite/gas/i386/gmi.d
@@ -0,0 +1,12 @@ 
+#objdump: -dw
+#name: zhaoxin gmi
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+000 <foo>:
+   0:[	 ]*f2 0f a6 c0 [	 ]*sm2
+   4:[	 ]*f3 0f a6 e8 [	 ]*sm3
+   8:[	 ]*f3 0f a7 f0 [	 ]*sm4
+#pass
diff --git a/gas/testsuite/gas/i386/gmi.s b/gas/testsuite/gas/i386/gmi.s
new file mode 100644
index 00000000000..c412de55d31
--- /dev/null
+++ b/gas/testsuite/gas/i386/gmi.s
@@ -0,0 +1,8 @@ 
+# ZHAOXIN GMI instructions
+
+	.text
+foo:
+	sm2
+	sm3
+	sm4
+	.p2align 4,0
diff --git a/gas/testsuite/gas/i386/i386.exp b/gas/testsuite/gas/i386/i386.exp
index 699e2004bef..4aaa2239069 100644
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -114,6 +114,7 @@  if [gas_32_check] then {
     run_dump_test "quoted2"
     run_dump_test "unary"
     run_dump_test "padlock"
+    run_dump_test "gmi"
     run_dump_test "crx"
     run_list_test "cr-err" ""
     run_dump_test "cdr"
diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c
index 3a4af4d61a3..e012937e303 100644
--- a/opcodes/i386-dis.c
+++ b/opcodes/i386-dis.c
@@ -90,6 +90,7 @@  static bool PCLMUL_Fixup (instr_info *, int, int);
 static bool VPCMP_Fixup (instr_info *, int, int);
 static bool VPCOM_Fixup (instr_info *, int, int);
 static bool NOP_Fixup (instr_info *, int, int);
+static bool MONTMUL_Fixup (instr_info *, int, int);
 static bool OP_3DNowSuffix (instr_info *, int, int);
 static bool CMP_Fixup (instr_info *, int, int);
 static bool REP_Fixup (instr_info *, int, int);
@@ -1050,6 +1051,9 @@  enum
   PREFIX_0F7D,
   PREFIX_0F7E,
   PREFIX_0F7F,
+  PREFIX_0FA6_REG_0_MOD_3,
+  PREFIX_0FA6_REG_5_MOD_3,
+  PREFIX_0FA7_REG_6_MOD_3,
   PREFIX_0FAE_REG_0_MOD_3,
   PREFIX_0FAE_REG_1_MOD_3,
   PREFIX_0FAE_REG_2_MOD_3,
@@ -2850,9 +2854,12 @@  static const struct dis386 reg_table[][8] = {
   },
   /* REG_0FA6 */
   {
-    { "montmul",	{ { OP_0f07, 0 } }, 0 },
+    { PREFIX_TABLE (PREFIX_0FA6_REG_0_MOD_3) },
     { "xsha1",		{ { OP_0f07, 0 } }, 0 },
     { "xsha256",	{ { OP_0f07, 0 } }, 0 },
+    { Bad_Opcode },
+    { Bad_Opcode },
+    { PREFIX_TABLE (PREFIX_0FA6_REG_5_MOD_3) },
   },
   /* REG_0FA7 */
   {
@@ -2862,6 +2869,7 @@  static const struct dis386 reg_table[][8] = {
     { "xcrypt-ctr",	{ { OP_0f07, 0 } }, 0 },
     { "xcrypt-cfb",	{ { OP_0f07, 0 } }, 0 },
     { "xcrypt-ofb",	{ { OP_0f07, 0 } }, 0 },
+    { PREFIX_TABLE (PREFIX_0FA7_REG_6_MOD_3) },
   },
   /* REG_0FAE */
   {
@@ -3438,6 +3446,30 @@  static const struct dis386 prefix_table[][4] = {
     { "movdqa",	{ EXxS, XM }, PREFIX_OPCODE },
   },
 
+  /* PREFIX_0FA6_REG_0_MOD_3 */
+  {
+    { Bad_Opcode },
+    { "montmul",	{ { MONTMUL_Fixup, 0 } }, 0},
+    { Bad_Opcode },
+    { "sm2",	{ Skip_MODRM }, 0 },
+  },
+
+  /* PREFIX_0FA6_REG_5_MOD_3 */
+  {
+    { Bad_Opcode },
+    { "sm3",	{ Skip_MODRM }, 0 },
+    { Bad_Opcode },
+    { Bad_Opcode },
+  },
+
+  /* PREFIX_0FA7_REG_6_MOD_3 */
+  {
+    { Bad_Opcode },
+    { "sm4",	{ Skip_MODRM }, 0 },
+    { Bad_Opcode },
+    { Bad_Opcode },
+  },
+
   /* PREFIX_0FAE_REG_0_MOD_3 */
   {
     { Bad_Opcode },
@@ -13086,6 +13118,21 @@  OP_0f07 (instr_info *ins, int bytemode, int sizeflag)
   return OP_E (ins, bytemode, sizeflag);
 }
 
+/* montmul instruction need display repz and skip modrm */
+
+static bool
+MONTMUL_Fixup (instr_info *ins, int bytemode, int sizeflag)
+{
+  (void) bytemode;
+  (void) sizeflag;
+  /* The 0xf3 prefix should be displayed as "repz" for montmul. */
+  if (ins->prefixes & PREFIX_REPZ)
+    ins->all_prefixes[ins->last_repz_prefix] = 0xf3;
+  ins->codep++;
+  ins->has_skipped_modrm = true;
+  return true;
+}
+
 /* NOP is an alias of "xchg %ax,%ax" in 16bit mode, "xchg %eax,%eax" in
    32bit mode and "xchg %rax,%rax" in 64bit mode.  */
 
diff --git a/opcodes/i386-gen.c b/opcodes/i386-gen.c
index 565aae722f8..0f7ab4c6a56 100644
--- a/opcodes/i386-gen.c
+++ b/opcodes/i386-gen.c
@@ -334,6 +334,7 @@  static bitfield cpu_flags[] =
   BITFIELD (3dnow),
   BITFIELD (3dnowA),
   BITFIELD (PadLock),
+  BITFIELD (GMI),
   BITFIELD (SVME),
   BITFIELD (VMX),
   BITFIELD (SMX),
diff --git a/opcodes/i386-opc.h b/opcodes/i386-opc.h
index c0d5e44d461..d404fbc6b05 100644
--- a/opcodes/i386-opc.h
+++ b/opcodes/i386-opc.h
@@ -66,6 +66,8 @@  enum i386_cpu
   CpuSSE3,
   /* VIA PadLock required */
   CpuPadLock,
+  /* ZHAOXIN GMI required */
+  CpuGMI,
   /* AMD Secure Virtual Machine Ext-s required */
   CpuSVME,
   /* VMX Instructions required */
@@ -400,6 +402,7 @@  typedef union i386_cpu_flags
       unsigned int cpusse2:1;
       unsigned int cpusse3:1;
       unsigned int cpupadlock:1;
+      unsigned int cpugmi:1;
       unsigned int cpusvme:1;
       unsigned int cpuvmx:1;
       unsigned int cpusmx:1;
diff --git a/opcodes/i386-opc.tbl b/opcodes/i386-opc.tbl
index 9a14a4d1819..7ded4e64133 100644
--- a/opcodes/i386-opc.tbl
+++ b/opcodes/i386-opc.tbl
@@ -2131,6 +2131,11 @@  xcryptofb, 0xf30fa7e8, PadLock, NoSuf|RepPrefixOk, {}
 // Alias for xstore-rng.
 xstore, 0xfa7c0, PadLock, NoSuf|RepPrefixOk, {}
 
+// ZHAOXIN GMI instructions
+sm2, 0xf20fa6c0, GMI, NoSuf, {}
+sm3, 0xf30fa6e8, GMI, NoSuf, {}
+sm4, 0xf30fa7f0, GMI, NoSuf, {}
+
 // Multy-precision Add Carry, rdseed instructions.
 <adx:pfx, c:66, o:f3>
 ad<adx>x, 0x<adx:pfx>66, ADX&APX_F, C|Modrm|CheckOperandSize|No_bSuf|No_wSuf|No_sSuf|DstVVVV|EVexMap4, { Reg32|Reg64|Unspecified|BaseIndex, Reg32|Reg64, Reg32|Reg64 }
-- 
2.27.0