Message-ID: <08385b8c-0fae-3249-3dfb-5a7800928213@amd.com>
Date:   Mon, 30 Jan 2023 16:04:58 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [PATCH v3 1/2] perf script: Show branch speculation info
To:     linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
        x86@kernel.org
Cc:     peterz@infradead.org, bp@alien8.de, acme@kernel.org,
        namhyung@kernel.org, jolsa@kernel.org, tglx@linutronix.de,
        mingo@redhat.com, mark.rutland@arm.com,
        alexander.shishkin@linux.intel.com, dave.hansen@linux.intel.com,
        james.clark@arm.com, irogers@google.com, eranian@google.com,
        ananth.narayan@amd.com, ravi.bangoria@amd.com,
        santosh.shukla@amd.com
References: <cover.1675057032.git.sandipan.das@amd.com>
 <87c48949f5a442dda6d4dc75d4a4303973eb04b2.1675057032.git.sandipan.das@amd.com>
Content-Language: en-US
From:   Sandipan Das <sandipan.das@amd.com>
In-Reply-To: <87c48949f5a442dda6d4dc75d4a4303973eb04b2.1675057032.git.sandipan.das@amd.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dk4vR25VZkhqU0VZOGZmOGQ1eEpRZHBjOVF1TzNBRDRGenRlN3lZdmpVSkJY?=
 =?utf-8?B?cWlKeCtrZkE2NE5JdWJUeENGSGVxTlFFK014VS8wY25Zb1d0SGttTGZvZkdY?=
 =?utf-8?B?VWVwblMvczFubG9mTmFoYkJHT0hxaG9pTzVCMUQvaHJXeER6T3lOYXduWTlw?=
 =?utf-8?B?eDFZT0NRQ2dNVVFmVXpvclZhVGJHMVVPM1N6SlZLT0IrMWZ5SmJiNlVjdEdT?=
 =?utf-8?B?cEZIb3NYWjB4TEsrMEIvb1dvR0JQbWMyUUE1QzM5SUlRVkFCVndrNkl1bjgv?=
 =?utf-8?B?emZGVEs2T3BrOHdqR3E3T0UweHh6aE1nS1lKMTNjdElUVjBQWitKNkZXYndB?=
 =?utf-8?B?dzJrandCVlVMNzNuR25RbnUwamRiaFRCdEJRSnpBdVNhNXF4c0drTTZBRDZD?=
 =?utf-8?B?dHkyeDM4cFpBd20rdURPOUE1alJIR3EzdDExVmdWZ3AveFNKRDB2NUNVU0Ir?=
 =?utf-8?B?WDc3THpxdm1KL3hrdGhHcWxIdkdwUEsxMW02dzNPU3cwekwvSVpIeE02eDdy?=
 =?utf-8?B?NW9BQkdZbGVPN3BKUDlBRGJSWmNhZHdmN1g3YXZQdUhqaFpSTEg2Wk9uTjdO?=
 =?utf-8?B?YWJsUEtLSldTNDdpc2pVNmNMUm1NYWZLUXEyclR3djJrS2ZVaHZHZnViRG9n?=
 =?utf-8?B?WWY4ZWNmbW9tRkh4M1paUzZQRUFISVY5Mm9ScGVWL3RPemF5VnplQ052emdQ?=
 =?utf-8?B?bnRINkZUVFNGTjIyRXIzM1ZleUlzcG5OUk5hbGlpSlgwYUJmd256OE5OTmo4?=
 =?utf-8?B?T0RaNUpWdzlHdFFiVHBQRXI5Q1FQY3BQQ0kyMkNPRFFGM1BRemR6TkNOc0NP?=
 =?utf-8?B?L0xURnJxWkFpcjdtekhnRFg5eTVCcCtsVVE0cEVyQzdUWDk1MWs4dUtnd29M?=
 =?utf-8?B?VXJXOE1qaDhVSUp3cDhCVGl4WTVrSlpMd0Z4THVSUStwVllMMVRONmlxTXdC?=
 =?utf-8?B?RGlxUVAySElUd2EzQm5XVjBPbWtqR3JJZkpwcUV2bXYxTW5XdGUwL0pYMGd1?=
 =?utf-8?B?eUFMN2tmTDI5dW5YczM2cWk0QVI2eXl1WGlzS0owMEtvZTZBK29QOUlPWUp6?=
 =?utf-8?B?bG11U2huWGVmMDJ6RnVaV0loWWtKTnU0UkQ4akgySWdmSmVFbzRIeUhHdWp3?=
 =?utf-8?B?THhzRGlEdlU5MmFsNnlZRGpWUWdqSzdsNUxCNTM3cCt1RkZSdUVVeUZWdDdq?=
 =?utf-8?B?QjN1Z1Y4ZWNtRnJBNDYvTzBTOFc3QlZQRk5xMWFKVTlvT3lLZGJkMHJOQUh1?=
 =?utf-8?B?eTNEZHV2bU5MQVhkOXVIK2Jheklnelp5TlZianJVeDZ6bjY0OFdtUkpHMXdQ?=
 =?utf-8?B?dEJ6OHNtSFBSd0ZJOWV6WTNOdDU0eGk4Z0gvYVplNXBJOEdiWnh1L0RtNmkx?=
 =?utf-8?B?K05DcGxsazZheTNkTUlxTTZ5TGRsejRENXM4cGRzMzFNOW8xVGVUUnQwRVRz?=
 =?utf-8?B?dFpvekFGaHBsR1hEL2YxdmZNbWxKSHVVRk4wQjhVWjd2MDFvOFJxbHFhOGxz?=
 =?utf-8?B?UVJSRU5ZVG9BMHRhb0p4QmhjSkxqSEcwWTUrS2t0MVBJandUdjdZSXRqc0Ey?=
 =?utf-8?B?eDRNL2h1WEZUQzFXbW04SVRBa2J1M1dFeXZ2UTZWRDdTM0VDdThmUEFGbFFP?=
 =?utf-8?B?b3RjSGNsdk9zUkhtaERNNnRPRitDam01czNvZk14MUR2Q1hkcTY3TE9heTRt?=
 =?utf-8?B?aFhMcU9abTMya1k5Mmlnb3ZidDJnTnJRblQxNUNSUXBkSnpaa0R2U1NVWm5L?=
 =?utf-8?B?NHVJWllnZW56MmRaOVJBYnlXdmN0bEFHYndMejVjalVUY2dkQi9GQ1RTUm5N?=
 =?utf-8?B?cGU4emJ2SnJ3YURwL0ZXQSttT2tnV0Rid09HTTNkTVUvYjBZdGsza3owSDhI?=
 =?utf-8?B?YlZrUnRUWlE4WUJkNlUrMk1oRUs5aWRVdzhrNzlCR3pUajFpYzl5VW5xcEIx?=
 =?utf-8?B?eGhVTVRIeGRidEVoc3I4YUJZT0t1SWZ4T2FMVzQ5TXBZNXgvYlF0WU1yWWR6?=
 =?utf-8?B?R0g1djFkNFlzaUlzMU5tOFR1WTlqSEUrb2hWSHVKdGFDd0hHc0E3eWJVUFND?=
 =?utf-8?B?QmRVUDMyOU8ranpmSXVaanE0SmU5OFkyL21LeWpUdkhiZXNCY1RORUd4eU14?=
 =?utf-8?Q?D3ODkwdPKakQLExE6SGwUVW8Z?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 464fdb62-ad7a-47c5-71e7-08db02adab6c
X-MS-Exchange-CrossTenant-AuthSource: PH7PR12MB5712.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Jan 2023 10:35:13.3227
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: L8xtGqMM4Q+dFkkkvEe1bhJq6rUwN0VK9aVh3nNoS1ZHBzEi3/V00EmOkDshSkwiHWksioOxjpu2N5p5mxG/6g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR12MB8057
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 1/30/2023 11:29 AM, Sandipan Das wrote:
> Show the branch speculation info if provided by the branch recording
> hardware feature. This can be useful for optimizing code further.
> 
> The speculation info is appended to the end of the list of fields so any
> existing tools that use "/" as a delimiter for access fields via an index
> remain unaffected. Also show "-" instead of "N/A" when speculation info
> is unavailable because "/" is used as the field separator.
> 
> E.g.
> 
>   $ perf record -j any,u,save_type ./test_branch
>   $ perf script --fields brstacksym
> 
> Before:
> 
>   [...]
>   check_match+0x60/strcmp+0x0/P/-/-/0/CALL
>   do_lookup_x+0x3c5/check_match+0x0/P/-/-/0/CALL
>   [...]
> 
> After:
> 
>   [...]
>   check_match+0x60/strcmp+0x0/P/-/-/0/CALL/NON_SPEC_CORRECT_PATH
>   do_lookup_x+0x3c5/check_match+0x0/P/-/-/0/CALL/NON_SPEC_CORRECT_PATH
>   [...]
> 
> Signed-off-by: Sandipan Das <sandipan.das@amd.com>
> ---
>  tools/perf/builtin-script.c |  5 +++--
>  tools/perf/util/branch.c    | 15 +++++++++++++++
>  tools/perf/util/branch.h    |  2 ++
>  tools/perf/util/evsel.c     | 15 ++++++++++++---
>  4 files changed, 32 insertions(+), 5 deletions(-)
> 

Sorry but I realized later that this change breaks the builtin branch test.
The additional change below fixes that.

diff --git a/tools/perf/tests/shell/test_brstack.sh b/tools/perf/tests/shell/test_brstack.sh
index 59195eb80052..1c49d8293003 100755
--- a/tools/perf/tests/shell/test_brstack.sh
+++ b/tools/perf/tests/shell/test_brstack.sh
@@ -30,14 +30,14 @@ test_user_branches() {
        #       brstack_foo+0x14/brstack_bar+0x40/P/-/-/0/CALL

        set -x
-       grep -E -m1 "^brstack_bench\+[^ ]*/brstack_foo\+[^ ]*/IND_CALL$"        $TMPDIR/perf.script
-       grep -E -m1 "^brstack_foo\+[^ ]*/brstack_bar\+[^ ]*/CALL$"      $TMPDIR/perf.script
-       grep -E -m1 "^brstack_bench\+[^ ]*/brstack_foo\+[^ ]*/CALL$"    $TMPDIR/perf.script
-       grep -E -m1 "^brstack_bench\+[^ ]*/brstack_bar\+[^ ]*/CALL$"    $TMPDIR/perf.script
-       grep -E -m1 "^brstack_bar\+[^ ]*/brstack_foo\+[^ ]*/RET$"               $TMPDIR/perf.script
-       grep -E -m1 "^brstack_foo\+[^ ]*/brstack_bench\+[^ ]*/RET$"     $TMPDIR/perf.script
-       grep -E -m1 "^brstack_bench\+[^ ]*/brstack_bench\+[^ ]*/COND$"  $TMPDIR/perf.script
-       grep -E -m1 "^brstack\+[^ ]*/brstack\+[^ ]*/UNCOND$"            $TMPDIR/perf.script
+       grep -E -m1 "^brstack_bench\+[^ ]*/brstack_foo\+[^ ]*/IND_CALL/.*$"     $TMPDIR/perf.script
+       grep -E -m1 "^brstack_foo\+[^ ]*/brstack_bar\+[^ ]*/CALL/.*$"   $TMPDIR/perf.script
+       grep -E -m1 "^brstack_bench\+[^ ]*/brstack_foo\+[^ ]*/CALL/.*$" $TMPDIR/perf.script
+       grep -E -m1 "^brstack_bench\+[^ ]*/brstack_bar\+[^ ]*/CALL/.*$" $TMPDIR/perf.script
+       grep -E -m1 "^brstack_bar\+[^ ]*/brstack_foo\+[^ ]*/RET/.*$"            $TMPDIR/perf.script
+       grep -E -m1 "^brstack_foo\+[^ ]*/brstack_bench\+[^ ]*/RET/.*$"  $TMPDIR/perf.script
+       grep -E -m1 "^brstack_bench\+[^ ]*/brstack_bench\+[^ ]*/COND/.*$"       $TMPDIR/perf.script
+       grep -E -m1 "^brstack\+[^ ]*/brstack\+[^ ]*/UNCOND/.*$"         $TMPDIR/perf.script
        set +x

        # some branch types are still not being tested:
@@ -57,7 +57,7 @@ test_filter() {

        # fail if we find any branch type that doesn't match any of the expected ones
        # also consider UNKNOWN branch types (-)
-       if grep -E -vm1 "^[^ ]*/($expect|-|( *))$" $TMPDIR/perf.script; then
+       if grep -E -vm1 "^[^ ]*/($expect|-|( *))/.*$" $TMPDIR/perf.script; then
                return 1
        fi
 }


- Sandipan