Hi Ulrich and community,

Please find attached the patch with the newly suggested changes with indentations as per GNU style/guidelines.

Thank you for guiding about the same. Kindly see my reply to 2 of your queries in blue.

Kindly push this patch if there are no changes.

Have a nice day ahead.

Thanks and regards,
Aditya.

>You've added this under the GDB 11 news section, which is wrong.

>You should instead add a new section
 > * Removed targets and native configurations
>in the topmost
>  *** Changes since GDB 13
>section.

This is done. Kindly let me know if this is okay.
>+    memset(&vmx, 0, sizeof(__vmx_context_t));
>+    if (data->arch64)

>Why is this check necessary, and what happens on 32-bit targets?

Thank you for reminding me. So I was experimenting with the altivec-regs.exp and site.exp in 32 bit mode attempting to see what happens if I have that condition. I forgot to remove it.

From: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>
Date: Tuesday, 7 March 2023 at 3:55 PM
To: gdb-patches@sourceware.org <gdb-patches@sourceware.org>, Aditya Kamath1 <Aditya.Kamath1@ibm.com>, simark@simark.ca <simark@simark.ca>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: Re: [PATCH] Enable vector instruction debugging for AIX
Aditya Kamath1 <Aditya.Kamath1@ibm.com> wrote:

>So I connected with my seniors on this. So AIX folks are using 7.1 now.
>So I have changed this to minimum 7.x.. Kindly see it in this patch.
>For those who are below 7.1, we are not supporting.

OK, this makes sense to me.  But it looks like you've added this
to the wrong section now, see below.

>>Fix coding style ({ should be indented).
>This is done. I have eliminated all RHS space and aligned the flower brackets wherever I saw it was not aligned.

Unfortunately it seems you changed many of these
in the wrong direction now.  GNU coding style is
to indent the { by two spaces, and then indent the
block inside by *another* two spaces:

  if (condition)
    {
      /* code */
    }

Also, the patch still introduces various instances of
8 spaces (that should be tabs), and whitespace at the
end of the line (which should be removed).


>@@ -938,6 +938,9 @@ info sources
>
> ARM Symbian                   arm*-*-symbianelf*
>
>+GDB no longer supports AIX 4.x, AIX 5.x and AIX 6.x.  The minimum supported
>+AIX version is now AIX 7.1.
>+

You've added this under the GDB 11 news section, which is wrong.

You should instead add a new section
  * Removed targets and native configurations
in the topmost
  *** Changes since GDB 13
section.

>+  __vmx_context_t vmx;
>+  if (__power_vmx() && (flags & PTHDB_FLAG_REGS))
>+  {

Wrong indentation.

>+  /* vsx registers.  */
>+  __vsx_context_t vsx;
>+  if (__power_vsx() && (flags & PTHDB_FLAG_REGS))
>+  {

Likewise.

>+  /* vector registers.  */
>+  if (__power_vmx() && (flags & PTHDB_FLAG_REGS))
>+  {

Likewise.

>+  /* vsx registers.  */
>+  if (__power_vsx() && (flags & PTHDB_FLAG_REGS))
>+  {

Likewise.

>+  /* Fill altivec-registers.  */
>+
>+  if (__power_vmx())
>+  {

Likewise throughout the block

>+    memset(&vmx, 0, sizeof(__vmx_context_t));
>+    if (data->arch64)

Why is this check necessary, and what happens on 32-bit targets?

>+  /* Fill vsx registers. */
>+
>+  if (__power_vsx())
>+  {
>+    memset(&vsx, 0, sizeof(__vsx_context_t));
>+    if (data->arch64)

Same question, also same indentation issues.

>+    /* Vector registers.  */
>+    if (tdep->ppc_vr0_regnum != -1 && tdep->ppc_vrsave_regnum != -1
>+      && (regno == -1 || (regno >= tdep->ppc_vr0_regnum
>+      && regno <= tdep->ppc_vrsave_regnum)))
>+    {

Also indentation issues throughout the block.

>+    /* VSX registers.  */
>+    if (tdep->ppc_vsr0_upper_regnum != -1 && (regno == -1
>+      || (regno >=tdep->ppc_vsr0_upper_regnum)))

You've now removed the upper limit completely.  I think this needs an
  && regno < tdep->ppc_vsr0_upper_regnum + ppc_num_vshrs

>+  memset(&vsx, 0, sizeof(__vsx_context_t));
>+  if (__power_vsx() && thrd_i > 0)
>+  {

Indentation.

>+  memset(&vmx, 0, sizeof(__vmx_context_t));
>+  if (__power_vmx() && thrd_i > 0)
>+  {

Likewise.

>+  memset(&vmx, 0, sizeof(__vmx_context_t));
>+  if (__power_vmx() && thrd_i > 0)
>+  {

Likewise.

>+  memset(&vsx, 0, sizeof(__vsx_context_t));
>+  if (__power_vsx() && thrd_i > 0)
>+  {

Likewise.

>+  /* Alti-vec register.  */
>+  if (altivec_register_p (gdbarch, regno))
>+  {
>+    fetch_altivec_registers_aix (regcache);
>+    return;
>+  }
>+
>+  /* VSX register.  */
>+  if (vsx_register_p (gdbarch, regno))
>+  {
>+    fetch_vsx_registers_aix (regcache);
>+    return;
>+  }

Likewise.

>+  if (altivec_register_p (gdbarch, regno))
>+  {
>+    store_altivec_register_aix (regcache, regno);
>+    return;
>+  }
>+
>+  if (vsx_register_p (gdbarch, regno))
>+  {
>+    store_vsx_register_aix (regcache, regno);
>+    return;
>+  }

Likewise.

>+const struct target_desc *
>+rs6000_nat_target::read_description ()
>+{
>+   if (ARCH64())
>+   {
>+     if (__power_vsx ())
>+       return tdesc_powerpc_vsx64;
>+     else if (__power_vmx ())
>+       return tdesc_powerpc_altivec64;
>+   }
>+   else
>+   {
>+     if (__power_vsx ())
>+       return tdesc_powerpc_vsx32;
>+     else if (__power_vmx ())
>+       return tdesc_powerpc_altivec32;
>+   }
>+   return NULL;
>+}

Likewise.

>+  if (regnum == -1)
>+  {

Likewise.

>+  if (regnum == -1)
>+  {

Likewise.

>+  if (regnum == -1)
>+  {

Likewise.

>+  if (regnum == -1)
>+  {

Likewise.

Bye,
Ulrich