| Message ID | 1432619352-18785-1-git-send-email-crosthwaite.peter@gmail.com |
|---|---|
| State | New |
| Headers | show |
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
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 >
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
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 >
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
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 >
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
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