Kugan Vivekanandarajah <[email protected]> writes:
> [sending again as the email seems to have not delivered]
>
> Hi Richard,
>
>> On 7 Jun 2025, at 1:12 am, Richard Sandiford <[email protected]>
>> wrote:
>>
>> External email: Use caution opening links or attachments
>>
>>
>> Jan Hubicka <[email protected]> writes:
>>>> Should I go with:
>>>>
>>>> +autofdo_target
>>>>
>>>> +autofdo_target="i386"
>>>> +case "${target}" in
>>>> + aarch64-*-*)
>>>> + autofdo_target="aarch64"
>>>> + ;;
>>>> +esac
>>>>
>>>> As in the first version? I can test and send a patch for review if there
>>>> is no other better alternative.
>>>
>>> This looks OK - I can not think of better alternative. At some point we
>>> need to translate the target CPU to directory name. The build machinery
>>> is not exactly my strong point, so perhaps someone with more knowledge
>>> can chime in.
>>
>> Can we at least keep the variable name as "cpu_type", rather than add
>> a new classification? I don't see a good reason why autofdo would need
>> to use its own private naming scheme, rather than sticking to the one
>> that gcc/configure* already uses (i.e. the one used for gcc/config/<cpu>).
>>
>> It is incovenient that the toplevel doesn't have access to the logic
>> used to set that variable though...
>>
>
> I changed it to:
> +# Special case cpu_type for x86_64 as it shares AUTO_PROFILE from i386.
> +if test "${cpu_type}" = "x86_64" ; then
> + cpu_type="i386"
> +fs
>
> Is this ok? Tested on x86_64 and aarch64 linux-gnu.
>
> Thanks,
> Kugan
>
> From 30445db0509089117bd03c70b0cc00656e79e1ab Mon Sep 17 00:00:00 2001
> From: Kugan Vivekanandarajah <[email protected]>
> Date: Fri, 6 Jun 2025 22:11:57 -0700
> Subject: [PATCH] [AutoFDO] Fix profile bootstrap for x86_64
>
> This patch fixes profile bootstrap for x86_64 by special
> caseing cpu_type for x86_64 as it shares AUTO_PROFILE
> from i386.
>
> ChangeLog:
>
> * configure.ac: Special case cpu_type for x86_64.
> * configure: Regenerate.
>
> Signed-off-by: Kugan Vivekanandarajah <[email protected]>
> ---
> configure | 4 ++++
> configure.ac | 4 ++++
> 2 files changed, 8 insertions(+)
>
> diff --git a/configure b/configure
> index ed1e5a4e818..8ec8a9f111b 100755
> --- a/configure
> +++ b/configure
> @@ -3397,6 +3397,10 @@ case "${target}" in
> esac
>
> cpu_type=`echo ${host} | sed 's/-.*$//'`
> +# Special case cpu_type for x86_64 as it shares AUTO_PROFILE from i386.
> +if test "${cpu_type}" = "x86_64" ; then
> + cpu_type="i386"
> +fi
>
>
> # Disable libssp for some systems.
> diff --git a/configure.ac b/configure.ac
> index 33c130f4e02..5fa101f77fd 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -622,6 +622,10 @@ case "${target}" in
> esac
>
> cpu_type=`echo ${host} | sed 's/-.*$//'`
> +# Special case cpu_type for x86_64 as it shares AUTO_PROFILE from i386.
I'd prefer something like:
# Map the first part of the triplet to the GCC config/<cpu> name.
# Many targets do not use this variable yet, so the logic is intentionally
# incomplete. If the variable becomes more widely used in future,
# we should probably look at trying to share code with gcc/config.gcc.
OK with that change, thanks.
Richard
> +if test "${cpu_type}" = "x86_64" ; then
> + cpu_type="i386"
> +fi
> AC_SUBST(cpu_type)
>
> # Disable libssp for some systems.
