Kugan Vivekanandarajah <kvivekana...@nvidia.com> writes: > diff --git a/Makefile.in b/Makefile.in > index b1ed67d3d4f..b5e3e520791 100644 > --- a/Makefile.in > +++ b/Makefile.in > @@ -4271,7 +4271,7 @@ all-stageautoprofile-bfd: configure-stageautoprofile-bfd > $(HOST_EXPORTS) \ > $(POSTSTAGE1_HOST_EXPORTS) \ > cd $(HOST_SUBDIR)/bfd && \ > - $$s/gcc/config/i386/$(AUTO_PROFILE) \ > + $$s/gcc/config/@cpu_type@/$(AUTO_PROFILE) \ > $(MAKE) $(BASE_FLAGS_TO_PASS) \ > CFLAGS="$(STAGEautoprofile_CFLAGS)" \ > GENERATOR_CFLAGS="$(STAGEautoprofile_GENERATOR_CFLAGS)" \
The usual style seems to be to assign @foo@ to a makefile variable called foo or FOO, rather than to use @foo@ directly in rules. Otherwise the makefile stuff looks good. I don't feel qualified to review the script, but some general shell stuff: > diff --git a/gcc/config/aarch64/gcc-auto-profile > b/gcc/config/aarch64/gcc-auto-profile > new file mode 100755 > index 00000000000..0ceec035e69 > --- /dev/null > +++ b/gcc/config/aarch64/gcc-auto-profile > @@ -0,0 +1,51 @@ > +#!/bin/sh > +# Profile workload for gcc profile feedback (autofdo) using Linux perf. > +# Copyright The GNU Toolchain Authors. > +# > +# This file is part of GCC. > +# > +# GCC is free software; you can redistribute it and/or modify it under > +# the terms of the GNU General Public License as published by the Free > +# Software Foundation; either version 3, or (at your option) any later > +# version. > + > +# GCC is distributed in the hope that it will be useful, but WITHOUT ANY > +# WARRANTY; without even the implied warranty of MERCHANTABILITY or > +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License > +# for more details. > + > +# You should have received a copy of the GNU General Public License > +# along with GCC; see the file COPYING3. If not see > +# <http://www.gnu.org/licenses/>. */ > + > +# Run perf record with branch stack sampling and check for > +# specific error message to see if it is supported. > +use_brbe=true > +output=$(perf record -j any,u ls 2>&1) How about using /bin/true rather than ls for the test program? > +if [[ "$output" = *"Error::P: PMU Hardware or event type doesn't support > branch stack sampling."* ]]; then [[ isn't POSIX, or at least dash doesn't accept it. Since this script is effectively linux-specific, we can probably assume that /bin/bash exists and use that in the #! line. If we use bash, then the test could use =~ rather than an exact match. This could be useful if perf prints other diagnostics besides the one being tested for, or if future versions of perf alter the wording slightly. > + use_brbe=false > +fi > + > +FLAGS=u > +if [ "$1" = "--kernel" ] ; then > + FLAGS=k > + shift > +fi > +if [ "$1" = "--all" ] ; then How about making this an elif, so that we don't accept --kernel --all? > + FLAGS=u,k > + shift > +fi > + > +if [ "$use_brbe" = true ] ; then > + if grep -q hypervisor /proc/cpuinfo ; then > + echo >&2 "Warning: branch profiling may not be functional in VMs" > + fi > + set -x > + perf record -j any,$FLAGS "$@" > + set +x > +else > + set -x > + echo >&2 "Warning: branch profiling may not be functional without BRBE" > + perf record "$@" > + set +x Putting the set -x after the echo seems better, as for the "then" branch. Thanks, Richard > +fi
