[PATCH] Restore the order of file lists in features/Makefile

[PATCH] Restore the order of file lists in features/Makefile

[Maciej W. Rozycki]

Expedite register and XML file lists used to be sorted alphabetically, 
but they have recently kept diverging more and more from that order. 
Using the alphabetic order improves legibility and reduces the risk of 
merge conflicts, which is very high if new entries are added at the end 
instead.

	gdb/
	* features/Makefile: Restore the alphabetic order of expedite
	register and XML file lists.
---
Hi,

 I think the goal list in WHICH was supposed to be alphabetic as well 
and has also diverged, but unlike these it has multiple entries per line 
and the change required will be somewhat more complex than mere line 
shuffling this patch consists of.  So I'll defer it to a separate patch
which I'll send if we agree we want to restore and then keep the order.

 OK to apply?

  Maciej

gdb-features-order.diff
Index: binutils/gdb/features/Makefile
===================================================================
--- binutils.orig/gdb/features/Makefile	2016-10-21 04:08:19.669732345 +0100
+++ binutils/gdb/features/Makefile	2016-10-21 04:09:45.471189710 +0100
@@ -81,15 +81,14 @@ WHICH = aarch64 \
 
 # Record which registers should be sent to GDB by default after stop.
 aarch64-expedite = x29,sp,pc
+amd64-expedite = rbp,rsp,rip
 arm-expedite = r11,sp,pc
 i386-expedite = ebp,esp,eip
-amd64-expedite = rbp,rsp,rip
-x32-expedite = rbp,rsp,rip
-mips-expedite = r29,pc
+microblaze-expedite = r1,rpc
 mips-dsp-expedite = r29,pc
-mips64-expedite = r29,pc
+mips-expedite = r29,pc
 mips64-dsp-expedite = r29,pc
-microblaze-expedite = r1,rpc
+mips64-expedite = r29,pc
 nios2-linux-expedite = sp,pc
 powerpc-expedite = r1,pc
 rs6000/powerpc-cell32l-expedite = r1,pc,r0,orig_r3,r4
@@ -101,20 +100,21 @@ s390-linux64-expedite = r14l,r15l,pswa
 s390-linux64v1-expedite = r14l,r15l,pswa
 s390-linux64v2-expedite = r14l,r15l,pswa
 s390-te-linux64-expedite = r14l,r15l,pswa
-s390-vx-linux64-expedite = r14l,r15l,pswa
 s390-tevx-linux64-expedite = r14l,r15l,pswa
+s390-vx-linux64-expedite = r14l,r15l,pswa
 s390x-linux64-expedite = r14,r15,pswa
 s390x-linux64v1-expedite = r14,r15,pswa
 s390x-linux64v2-expedite = r14,r15,pswa
 s390x-te-linux64-expedite = r14,r15,pswa
-s390x-vx-linux64-expedite = r14,r15,pswa
 s390x-tevx-linux64-expedite = r14,r15,pswa
-tic6x-c64xp-expedite = A15,PC
-tic6x-c64x-expedite = A15,PC
+s390x-vx-linux64-expedite = r14,r15,pswa
 tic6x-c62x-expedite = A15,PC
-tic6x-c64xp-linux-expedite = A15,PC
-tic6x-c64x-linux-expedite = A15,PC
 tic6x-c62x-linux-expedite = A15,PC
+tic6x-c64x-expedite = A15,PC
+tic6x-c64x-linux-expedite = A15,PC
+tic6x-c64xp-expedite = A15,PC
+tic6x-c64xp-linux-expedite = A15,PC
+x32-expedite = rbp,rsp,rip
 
 
 XSLTPROC = xsltproc
@@ -127,8 +127,8 @@ OUTPUTS = $(patsubst %,$(outdir)/%.dat,$
 # to make on the command line.
 XMLTOC = \
 	aarch64.xml \
-	arc-v2.xml \
 	arc-arcompact.xml \
+	arc-v2.xml \
 	arm/arm-with-iwmmxt.xml \
 	arm/arm-with-m-fpa-layout.xml \
 	arm/arm-with-m-vfp-d16.xml \
@@ -137,16 +137,18 @@ XMLTOC = \
 	arm/arm-with-vfpv2.xml \
 	arm/arm-with-vfpv3.xml \
 	i386/amd64-avx-linux.xml \
+	i386/amd64-avx-mpx-linux.xml \
+	i386/amd64-avx-mpx.xml \
 	i386/amd64-avx.xml \
 	i386/amd64-avx512-linux.xml \
 	i386/amd64-avx512.xml \
 	i386/amd64-linux.xml \
 	i386/amd64-mpx-linux.xml \
 	i386/amd64-mpx.xml \
-	i386/amd64-avx-mpx-linux.xml \
-	i386/amd64-avx-mpx.xml \
 	i386/amd64.xml \
 	i386/i386-avx-linux.xml \
+	i386/i386-avx-mpx-linux.xml \
+	i386/i386-avx-mpx.xml \
 	i386/i386-avx.xml \
 	i386/i386-avx512-linux.xml \
 	i386/i386-avx512.xml \
@@ -155,8 +157,6 @@ XMLTOC = \
 	i386/i386-mmx.xml \
 	i386/i386-mpx-linux.xml \
 	i386/i386-mpx.xml \
-	i386/i386-avx-mpx-linux.xml \
-	i386/i386-avx-mpx.xml \
 	i386/i386.xml \
 	i386/x32-avx-linux.xml \
 	i386/x32-avx.xml \
@@ -214,12 +214,12 @@ XMLTOC = \
 	s390-linux64v1.xml \
 	s390-linux64v2.xml \
 	s390-te-linux64.xml \
+	s390-tevx-linux64.xml \
+	s390-vx-linux64.xml \
 	s390x-linux64.xml \
 	s390x-linux64v1.xml \
 	s390x-linux64v2.xml \
 	s390x-te-linux64.xml \
-	s390-tevx-linux64.xml \
-	s390-vx-linux64.xml \
 	s390x-tevx-linux64.xml \
 	s390x-vx-linux64.xml \
 	tic6x-c62x-linux.xml \

[PING][PATCH] Restore the order of file lists in features/Makefile

[Maciej W. Rozycki]

On Fri, 28 Oct 2016, Maciej W. Rozycki wrote:

> 	gdb/
> 	* features/Makefile: Restore the alphabetic order of expedite
> 	register and XML file lists.

 Can I ask for <https://patchwork.sourceware.org/patch/16914/> to be
reviewed?

  Maciej

Re: [PING][PATCH] Restore the order of file lists in features/Makefile

[Simon Marchi]

On 2016-11-08 03:48, Maciej W. Rozycki wrote:
> On Fri, 28 Oct 2016, Maciej W. Rozycki wrote:
> 
>> 	gdb/
>> 	* features/Makefile: Restore the alphabetic order of expedite
>> 	register and XML file lists.
> 
>  Can I ask for <https://patchwork.sourceware.org/patch/16914/> to be
> reviewed?
> 
>   Maciej

I am in favor of this patch.  I also think we should clean up the file 
lists in the main gdb Makefile, to have one file per line and sorted 
alphabetically.

Simon

Re: [PATCH] Restore the order of file lists in features/Makefile

[Yao Qi]

On Fri, Oct 28, 2016 at 3:52 PM, Maciej W. Rozycki <macro@imgtec.com> wrote:
> Expedite register and XML file lists used to be sorted alphabetically,
> but they have recently kept diverging more and more from that order.
> Using the alphabetic order improves legibility and reduces the risk of
> merge conflicts, which is very high if new entries are added at the end
> instead.
>
>         gdb/
>         * features/Makefile: Restore the alphabetic order of expedite
>         register and XML file lists.

Could you add a line of comment on expedite register and XML files
that they should be sorted alphabetically?

Otherwise, patch is good to me.

-- 
Yao (齐尧)

Re: [PATCH] Restore the order of file lists in features/Makefile

[Pedro Alves]

On 10/28/2016 03:52 PM, Maciej W. Rozycki wrote:
>  # Record which registers should be sent to GDB by default after stop.
>  aarch64-expedite = x29,sp,pc
> +amd64-expedite = rbp,rsp,rip
>  arm-expedite = r11,sp,pc
>  i386-expedite = ebp,esp,eip
> -amd64-expedite = rbp,rsp,rip
> -x32-expedite = rbp,rsp,rip
> -mips-expedite = r29,pc

...

> +tic6x-c64x-linux-expedite = A15,PC
> +tic6x-c64xp-expedite = A15,PC
> +tic6x-c64xp-linux-expedite = A15,PC
> +x32-expedite = rbp,rsp,rip
>  

It's a bit of a shame that amd64, i386 and x32 get split up.
And they'll get further and further away as we add more entries.

How bad would it look if we said it's "mostly" sorted?  E.g.,

# Record which registers should be sent to GDB by default after
# stop.  Mostly alpha-sorted, except when it makes sense to group
# architectures.

amd64-expedite = rbp,rsp,rip
i386-expedite = ebp,esp,eip
x32-expedite = rbp,rsp,rip

aarch64-expedite = x29,sp,pc
arm-expedite = r11,sp,pc
...

Thanks,
Pedro Alves

Re: [PATCH] Restore the order of file lists in features/Makefile

[Pedro Alves]

On 11/08/2016 02:48 PM, Pedro Alves wrote:
> On 10/28/2016 03:52 PM, Maciej W. Rozycki wrote:
>>  # Record which registers should be sent to GDB by default after stop.
>>  aarch64-expedite = x29,sp,pc
>> +amd64-expedite = rbp,rsp,rip
>>  arm-expedite = r11,sp,pc
>>  i386-expedite = ebp,esp,eip
>> -amd64-expedite = rbp,rsp,rip
>> -x32-expedite = rbp,rsp,rip
>> -mips-expedite = r29,pc
> 
> ...
> 
>> +tic6x-c64x-linux-expedite = A15,PC
>> +tic6x-c64xp-expedite = A15,PC
>> +tic6x-c64xp-linux-expedite = A15,PC
>> +x32-expedite = rbp,rsp,rip
>>  
> 
> It's a bit of a shame that amd64, i386 and x32 get split up.
> And they'll get further and further away as we add more entries.
> 
> How bad would it look if we said it's "mostly" sorted?  E.g.,
> 
> # Record which registers should be sent to GDB by default after
> # stop.  Mostly alpha-sorted, except when it makes sense to group
> # architectures.
> 
> amd64-expedite = rbp,rsp,rip
> i386-expedite = ebp,esp,eip
> x32-expedite = rbp,rsp,rip
> 
> aarch64-expedite = x29,sp,pc
> arm-expedite = r11,sp,pc
> ...

Hmmm, the rs6000/ entries have an explicit dir name:

rs6000/powerpc-cell32l-expedite = r1,pc,r0,orig_r3,r4
rs6000/powerpc-cell64l-expedite = r1,pc,r0,orig_r3,r4

Maybe all we need is to prefix the x86 ones with i386/ ?

i386/amd64-expedite = rbp,rsp,rip
i386/i386-expedite = ebp,esp,eip
i386/x32-expedite = rbp,rsp,rip

I don't know if that works.  I see that that prefix was actually
there recently, and it was removed by 414c838a722e recently.

Thanks,
Pedro Alves