[Qemu-devel] [PATCH] configure: Unify arm and aarch64 disas configury

[Qemu-devel] [PATCH] configure: Unify arm and aarch64 disas configury

[Peter Crosthwaite]

From: Peter Crosthwaite <crosthwaitepeter@gmail.com>

The "arm" variant for this case already contains everything needed
for aarch64. As aarch64 already uses arm as a base architecture, it
will already have the CONFIG_ARM_DIS defined meaning no functional
change. So just make the configure code simpler.

Following this patch:

$ ./configure --target-list=aarch64-softmmu,aarch64-linux-user
$ grep DIS aarch64-softmmu/config-target.mak
CONFIG_I386_DIS=y
CONFIG_ARM_DIS=y
CONFIG_ARM_A64_DIS=y
$ grep DIS aarch64-linux-user/config-target.mak
CONFIG_I386_DIS=y
CONFIG_ARM_DIS=y
CONFIG_ARM_A64_DIS=y

Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
Helps with multi-arch, less code paths to convert to multi-arch for
multi-arches many-way disas configury.
---
 configure | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/configure b/configure
index f758f32..3145dd6 100755
--- a/configure
+++ b/configure
@@ -5405,13 +5405,7 @@ for i in $ARCH $TARGET_BASE_ARCH ; do
     echo "CONFIG_ALPHA_DIS=y"  >> $config_target_mak
     echo "CONFIG_ALPHA_DIS=y"  >> config-all-disas.mak
   ;;
-  aarch64)
-    if test -n "${cxx}"; then
-      echo "CONFIG_ARM_A64_DIS=y"  >> $config_target_mak
-      echo "CONFIG_ARM_A64_DIS=y"  >> config-all-disas.mak
-    fi
-  ;;
-  arm)
+  arm|aarch64)
     echo "CONFIG_ARM_DIS=y"  >> $config_target_mak
     echo "CONFIG_ARM_DIS=y"  >> config-all-disas.mak
     if test -n "${cxx}"; then
-- 
1.9.1

Re: [Qemu-devel] [PATCH] configure: Unify arm and aarch64 disas configury

[Peter Maydell]

On 26 May 2015 at 06:49, Peter Crosthwaite <crosthwaitepeter@gmail.com> wrote:
> From: Peter Crosthwaite <crosthwaitepeter@gmail.com>
>
> The "arm" variant for this case already contains everything needed
> for aarch64. As aarch64 already uses arm as a base architecture, it
> will already have the CONFIG_ARM_DIS defined meaning no functional
> change. So just make the configure code simpler.

I'm wondering why we needed to put the A64 disassembler into
the "arm" variant at all...

-- PMM

Re: [Qemu-devel] [PATCH] configure: Unify arm and aarch64 disas configury

[Peter Crosthwaite]

On Tue, May 26, 2015 at 12:18 AM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 26 May 2015 at 06:49, Peter Crosthwaite <crosthwaitepeter@gmail.com> wrote:
>> From: Peter Crosthwaite <crosthwaitepeter@gmail.com>
>>
>> The "arm" variant for this case already contains everything needed
>> for aarch64. As aarch64 already uses arm as a base architecture, it
>> will already have the CONFIG_ARM_DIS defined meaning no functional
>> change. So just make the configure code simpler.
>
> I'm wondering why we needed to put the A64 disassembler into
> the "arm" variant at all...
>

That probably works too. I'm trying to get rid of the dup. respin it?

Regards,
Peter

> -- PMM
>

Re: [Qemu-devel] [PATCH] configure: Unify arm and aarch64 disas configury

[Peter Maydell]

On 26 May 2015 at 09:01, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> On Tue, May 26, 2015 at 12:18 AM, Peter Maydell
> <peter.maydell@linaro.org> wrote:
>> On 26 May 2015 at 06:49, Peter Crosthwaite <crosthwaitepeter@gmail.com> wrote:
>>> From: Peter Crosthwaite <crosthwaitepeter@gmail.com>
>>>
>>> The "arm" variant for this case already contains everything needed
>>> for aarch64. As aarch64 already uses arm as a base architecture, it
>>> will already have the CONFIG_ARM_DIS defined meaning no functional
>>> change. So just make the configure code simpler.
>>
>> I'm wondering why we needed to put the A64 disassembler into
>> the "arm" variant at all...
>>
>
> That probably works too. I'm trying to get rid of the dup. respin it?

I think it would look more like the other entries in the case statement,
so worth trying. Maybe we'll find out why it's done this way :-)

-- PMM

Re: [Qemu-devel] [PATCH] configure: Unify arm and aarch64 disas configury

[Peter Crosthwaite]

On Tue, May 26, 2015 at 1:24 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 26 May 2015 at 09:01, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>> On Tue, May 26, 2015 at 12:18 AM, Peter Maydell
>> <peter.maydell@linaro.org> wrote:
>>> On 26 May 2015 at 06:49, Peter Crosthwaite <crosthwaitepeter@gmail.com> wrote:
>>>> From: Peter Crosthwaite <crosthwaitepeter@gmail.com>
>>>>
>>>> The "arm" variant for this case already contains everything needed
>>>> for aarch64. As aarch64 already uses arm as a base architecture, it
>>>> will already have the CONFIG_ARM_DIS defined meaning no functional
>>>> change. So just make the configure code simpler.
>>>
>>> I'm wondering why we needed to put the A64 disassembler into
>>> the "arm" variant at all...
>>>
>>
>> That probably works too. I'm trying to get rid of the dup. respin it?
>
> I think it would look more like the other entries in the case statement,
> so worth trying. Maybe we'll find out why it's done this way :-)
>

OK I am at the bottom of it. The case statement only handles the base
arch and the host arch not the actual target arch. This means the
"arm)" case is all that is called for aarch64 target. the "aarch64)"
case in existing code is presumably to handle an AArch64 host. So the
current code will produce the minimal working configury. My patch will
still work, but will unneedingly add AArch32 disas to AA64 host. A
solution would be to patch that loop to go through all of the base
arch, target arch and host arch. Anyone know of a reason not to do so?

Regards,
Peter

> -- PMM
>

Re: [Qemu-devel] [PATCH] configure: Unify arm and aarch64 disas configury

[Peter Maydell]

On 7 June 2015 at 09:49, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> OK I am at the bottom of it. The case statement only handles the base
> arch and the host arch not the actual target arch.

Aha.

> This means the
> "arm)" case is all that is called for aarch64 target. the "aarch64)"
> case in existing code is presumably to handle an AArch64 host. So the
> current code will produce the minimal working configury. My patch will
> still work, but will unneedingly add AArch32 disas to AA64 host.

Presumably also the current code will unnecessarily add an A64
disassembler for all AArch32 host builds.

> A solution would be to patch that loop to go through all of the base
> arch, target arch and host arch. Anyone know of a reason not to do so?

Not offhand, but then I didn't figure out how the current code
worked when I looked at it :-)  We'd get duplicate lines in
the config files, but we already have those when host==target.

-- PMM

Re: [Qemu-devel] [PATCH] configure: Unify arm and aarch64 disas configury

[Peter Crosthwaite]

On Sun, Jun 7, 2015 at 3:51 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 7 June 2015 at 09:49, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>> OK I am at the bottom of it. The case statement only handles the base
>> arch and the host arch not the actual target arch.
>
> Aha.
>
>> This means the
>> "arm)" case is all that is called for aarch64 target. the "aarch64)"
>> case in existing code is presumably to handle an AArch64 host. So the
>> current code will produce the minimal working configury. My patch will
>> still work, but will unneedingly add AArch32 disas to AA64 host.
>
> Presumably also the current code will unnecessarily add an A64
> disassembler for all AArch32 host builds.
>
>> A solution would be to patch that loop to go through all of the base
>> arch, target arch and host arch. Anyone know of a reason not to do so?
>
> Not offhand, but then I didn't figure out how the current code
> worked when I looked at it :-)  We'd get duplicate lines in
> the config files, but we already have those when host==target.
>

Brute force it and sort -u? sort is considered a coreutil by at least
my manpages so I wonder if we can assume it is universally available.

Regards,
Peter

> -- PMM
>

Re: [Qemu-devel] [PATCH] configure: Unify arm and aarch64 disas configury

[Peter Maydell]

On 7 June 2015 at 20:19, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> On Sun, Jun 7, 2015 at 3:51 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> worked when I looked at it :-)  We'd get duplicate lines in
>> the config files, but we already have those when host==target.
>>
>
> Brute force it and sort -u? sort is considered a coreutil by at least
> my manpages so I wonder if we can assume it is universally available.

It's only a minor cosmetic thing in a file nobody looks at, so I'm
not sure it's worth the effort.

(We already use 'sort -u' in our Makefiles, incidentally.)

-- PMM