RE: [PATCH v2 1/4] x86: zap value-less Disp8MemShift from non-EVEX templates

["Cui, Lili" <lili.cui@intel.com>]

> In order to allow to continue to use templatized SSE2AVX templates when
> enhancing those to also cover eGPR usage, Disp8MemShift wants using to
> deviate from what general template attributes supply. That requires using
> Disp8MemShift in a way also affecting non-EVEX templates, yet having this
> attribute set would so far implicitly mean EVEX encoding.
> Recognize the case and instead zap the attribute if no other attribute
> indicates EVEX encoding.
> 

I'm confused about this patch, is it related to the movsd template? You removed the "Masking" for it and only left Disp8MemShift, but I thought it still belongs to EVEX template. 

+movsd, 0xf210, AVX512F, D|Modrm|EVexLIG|Space0F|VexW1|Disp8MemShift=3|NoSuf|SSE2AVX, { Qword|Unspecified|BaseIndex, RegXMM }

> No change in generated tables.
> ---
> This could be folded into the subsequent "x86/APX: extend SSE2AVX
> coverage", but I think it's better kept separate for being a little subtle.
> 
> --- a/opcodes/i386-gen.c
> +++ b/opcodes/i386-gen.c
> @@ -1126,6 +1126,7 @@ process_i386_opcode_modifier (FILE *tabl
>  			      char **opnd, int lineno, bool rex2_disallowed)  {
>    char *str, *next, *last;
> +  bool disp8_shift_derived = false;
>    bitfield modifiers [ARRAY_SIZE (opcode_modifiers)];
>    static const char *const spaces[] = {  #define SPACE(n) [SPACE_##n] = #n
> @@ -1190,7 +1191,10 @@ process_i386_opcode_modifier (FILE *tabl
>  	      if (strcasecmp(str, "Broadcast") == 0)
>  		val = get_element_size (opnd, lineno) + BYTE_BROADCAST;
>  	      else if (strcasecmp(str, "Disp8MemShift") == 0)
> -		val = get_element_size (opnd, lineno);
> +		{
> +		  val = get_element_size (opnd, lineno);
> +		  disp8_shift_derived = true;
> +		}
> 
>  	      set_bitfield (str, modifiers, val, ARRAY_SIZE (modifiers),
>  			    lineno);
> @@ -1243,13 +1247,21 @@ process_i386_opcode_modifier (FILE *tabl
> 
>    /* Rather than evaluating multiple conditions at runtime to determine
>       whether an EVEX encoding is being dealt with, derive that information
> -     right here.  A missing EVex attribute means "dynamic".  */
> -  if (!modifiers[EVex].value
> -      && (modifiers[Disp8MemShift].value
> -	  || modifiers[Broadcast].value
> +     right here.  A missing EVex attribute means "dynamic".  There's one
> +     exception though: A value-less Disp8MemShift needs zapping rather than
> +     respecting if no other attribute indicates EVEX encoding.  This is for
> +     certain SSE2AVX templatized templates to work reasonably.  */  if
> + (!modifiers[EVex].value)
> +    {
> +      if (modifiers[Broadcast].value
>  	  || modifiers[Masking].value
> -	  || modifiers[SAE].value))
> -    modifiers[EVex].value = EVEXDYN;
> +	  || modifiers[SAE].value)
> +	modifiers[EVex].value = EVEXDYN;
> +      else if (disp8_shift_derived)
> +	modifiers[Disp8MemShift].value = 0;
> +      else if (modifiers[Disp8MemShift].value)
> +	modifiers[EVex].value = EVEXDYN;
> +    }
> 
 
Why not just delete the Disp8MemShift? Maybe I missed something.

  if (!modifiers[EVex].value
      && (modifiers[Broadcast].value
          || modifiers[Masking].value
          || modifiers[SAE].value))
    modifiers[EVex].value = EVEXDYN;

Lili.