Re: [PATCH,V3] gas: x86: ginsn: adjust ginsns for certain lea ops

[Jan Beulich <jbeulich@suse.com>]

On 26.01.2024 10:00, Indu Bhagat wrote:
> @@ -5664,76 +5666,58 @@ x86_ginsn_lea (const symbolS *insn_end_sym)
>  {
>    offsetT src_disp = 0;
>    ginsnS *ginsn = NULL;
> -  unsigned int base_reg;
> -  unsigned int index_reg;
> +  unsigned int src1_reg;
> +  const reg_entry *src1;
>    offsetT index_scale;
>    unsigned int dst_reg;
>  
> -  if (!i.index_reg && !i.base_reg)
> +  if (!i.base_reg != !i.index_reg
> +      && (!i.index_reg || i.index_reg->reg_num != RegIZ))

Now this is odd - either you use the short form consistently, or you
don't use it at all. In the former case

  if (!i.base_reg != (!i.index_reg || i.index_reg->reg_num == RegIZ))

or

  if (!i.base_reg == (i.index_reg && i.index_reg->reg_num != RegIZ))

or yet some substantially similar expression (I hope I got the
transformations right, but in any event I expect you get the point).

>      {
> -      /* lea symbol, %rN.  */
> -      dst_reg = ginsn_dw2_regnum (i.op[1].regs);
> -      /* TBD_GINSN_INFO_LOSS - Skip encoding information about the symbol.  */
> -      ginsn = ginsn_new_mov (insn_end_sym, false,
> -			     GINSN_SRC_IMM, 0xf /* arbitrary const.  */, 0,
> -			     GINSN_DST_REG, dst_reg, 0);
> -    }
> -  else if (i.base_reg && !i.index_reg)
> -    {
> -      /* lea    -0x2(%base),%dst.  */
> -      base_reg = ginsn_dw2_regnum (i.base_reg);
> -      dst_reg = ginsn_dw2_regnum (i.op[1].regs);
> +      /* lea disp(%base), %dst    or    lea disp(,%index,imm), %dst.
> +	 But exclude the case when %index is RegIZ, else a GINSN_TYPE_MOV of
> +	 %rsi (GINSN_DW2_REGNUM_RSI_DUMMY) to %dst will be generated.  Such a
> +	 mov ginsn is imprecise, generate GINSN_TYPE_OTHER instead.  */

The text starting with "But ..." gives the impression that the comment
relates to subsequent code, where that further restriction would be
enforced. Yet this relates to the containing if(), so you may want to
make this more clear by e.g. adding "(as per above)". Maybe also with
tense changed: "But, as per above, with the case ... excluded."

Okay with suitable adjustments.

Jan