Re: [PATCH] Separate macro-fusible instructions from not for -mbranches-within-32B-boundaries

[Jan Beulich <jbeulich at suse dot com>]

On 02.03.2020 05:05, Hongtao Liu wrote:
> Update patch.

>+enum mf_jcc_kind
>+  {
>+    mf_jcc_jo = 0,  /* base opcode 0x70  */

Is there a reason for the " = 0" here? Without is, the standard
still mandates the value to be zero.

>+enum mf_cmp_kind
>+  {
>+    mf_cmp_test,  /* test */
>+    mf_cmp_cmp,  /* cmp */
>+    mf_cmp_and,  /* and */
>+    mf_cmp_alu,  /* add/sub */
>+    mf_cmp_incdec  /* inc/dec */
>+  };

I saw your reply to my question of why this is 5 instead of 3
enumerators. Yet I think this being a workaround for something
that hopefully will be fixed in newer hardware suggests to go
with the minimal necessary set first, and split enumerators
only if indeed needed down the road.

>@@ -8503,6 +8557,9 @@ add_branch_padding_frag_p (enum align_branch_kind *branch_p)
> 	}
>       else
> 	{
>+	  /* Because J<cc> and JN<cc> share same group in macro-fusible table,
>+	     igore the lowest bit.  */
>+	  *mf_jcc_p = (i.tm.base_opcode - 0x70) >> 1;

May I suggest to use (i.tm.base_opcode & 0x0e) >> 1, to be independent
of the insn variant presently in i.tm (after all we do dynamically
update it after copying from the template)?

Thanks for doing the other adjustments.

Jan