Kugan Vivekanandarajah <kvivekana...@nvidia.com> writes:
> [sending again as the email seems to have not delivered]
>
> Hi Richard,
>
>> On 7 Jun 2025, at 1:12 am, Richard Sandiford <richard.sandif...@arm.com> 
>> wrote:
>> 
>> External email: Use caution opening links or attachments
>> 
>> 
>> Jan Hubicka <hubi...@ucw.cz> 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 <kvivekana...@nvidia.com>
> 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 <kvivekana...@nvidia.com>
> ---
>  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.

Reply via email to