diff mbox

Restore the order of file lists in features/Makefile

Message ID alpine.DEB.2.00.1610281513350.31859@tp.orcam.me.uk
State New
Headers show

Commit Message

Maciej W. Rozycki Oct. 28, 2016, 2:52 p.m. UTC
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

Comments

Maciej W. Rozycki Nov. 8, 2016, 8:48 a.m. UTC | #1
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
Simon Marchi Nov. 8, 2016, 1:23 p.m. UTC | #2
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
Yao Qi Nov. 8, 2016, 2:37 p.m. UTC | #3
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.
Pedro Alves Nov. 8, 2016, 2:48 p.m. UTC | #4
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
Pedro Alves Nov. 8, 2016, 2:55 p.m. UTC | #5
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
diff mbox

Patch

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 \