From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id C0DC5385840D for ; Tue, 23 Apr 2024 15:44:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C0DC5385840D Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linux.ibm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org C0DC5385840D Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=148.163.156.1 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713887081; cv=none; b=PYAmBA/Av2ByicYywyg0aUG42cH2bVYIDzZirlIcYIKmsHBK7VrUgDEkfyuOhe8aOavV6vfZekUb1PSVvFGLLjqHp8KvUZ8smBbMMrs8ZdFvR/m0yl2K8xiZOKN5R9++JxgKS3dNWO8qg3P0RwAvfg2smk88sY4kIVD1+IGEncE= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713887081; c=relaxed/simple; bh=qn9hH/aIwKoTTxYL2n9k/mrvDNxumYfPAFZDcvaoyJI=; h=DKIM-Signature:Message-ID:Date:Subject:To:From:MIME-Version; b=Hok5FQKEFFSQHzLyzDzkyOL1Xg6At22yCzEEw3o1MTZqa6cv02nh1VBpIH6DT/fj6JyAHq7sIKoBVRUBJzmqJ7l+zBV/3R+B61+WJQAwkm45hVUyIJaSrpFva0wOjCvLU2EkSLDu0XNIUnzuGDf7b5LxoktTRDYHF4885rIMVzk= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from pps.filterd (m0353728.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 43NFWT0O017483; Tue, 23 Apr 2024 15:44:33 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : date : subject : to : cc : references : from : in-reply-to : content-type : content-transfer-encoding : mime-version; s=pp1; bh=Q6yWIGCCPYeWYfwHB8eA8oXejNOGmHy3t7lKAcdQAlU=; b=WXZeNwT3hPUvzIylq4uwslntfFA3eGcmauEH+IA7agf5hFhrCtlPndF+sX8vh23I8/aK CmVuaMZoknxoqC1OiJ/uhVfQFhhy5t1GbFQrrgrnwK3EzZ9HctZFRizQd/3HiqKIlBBC +v8W0OTaD1IUOImkMQxJh1gsyWDPN/FzFgdYu8CQezssBx4T0ImQh/5wgDugA/BKxro3 86fClazrkNnnQ2d7YCGdVvyUdHYqkyy7OhPGMBiKJtdn3LPD44SBBPBKcGyW1BL9n9Wi VSNo/DIYP7uOD1CqzOLpyn5fwXhir5cXPWsURegZL0IjNr874XOCGBWcKQWe3VlXgS61 3g== Received: from ppma12.dal12v.mail.ibm.com (dc.9e.1632.ip4.static.sl-reverse.com [50.22.158.220]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3xpfq7r0tt-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 23 Apr 2024 15:44:33 +0000 Received: from pps.filterd (ppma12.dal12v.mail.ibm.com [127.0.0.1]) by ppma12.dal12v.mail.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 43NDfsFi029898; Tue, 23 Apr 2024 15:44:32 GMT Received: from smtprelay05.fra02v.mail.ibm.com ([9.218.2.225]) by ppma12.dal12v.mail.ibm.com (PPS) with ESMTPS id 3xmr1ten9r-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 23 Apr 2024 15:44:32 +0000 Received: from smtpav06.fra02v.mail.ibm.com (smtpav06.fra02v.mail.ibm.com [10.20.54.105]) by smtprelay05.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 43NFiQG433882512 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 23 Apr 2024 15:44:28 GMT Received: from smtpav06.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 93BB920040; Tue, 23 Apr 2024 15:44:26 +0000 (GMT) Received: from smtpav06.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 3C28D20049; Tue, 23 Apr 2024 15:44:26 +0000 (GMT) Received: from [9.171.69.100] (unknown [9.171.69.100]) by smtpav06.fra02v.mail.ibm.com (Postfix) with ESMTP; Tue, 23 Apr 2024 15:44:26 +0000 (GMT) Message-ID: <16c9b3b9-5b65-4722-81b4-0578ffe3409c@linux.ibm.com> Date: Tue, 23 Apr 2024 17:44:25 +0200 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH 1/1] sframe: Represent FP without RA on stack To: Indu Bhagat , binutils@sourceware.org Cc: Andreas Krebbel References: <20240422155857.2497684-1-jremus@linux.ibm.com> <20240422155857.2497684-2-jremus@linux.ibm.com> <23e2d1d2-0637-46fa-ba5c-002f292e62c7@oracle.com> From: Jens Remus Content-Language: en-US Organization: IBM Deutschland Research & Development GmbH In-Reply-To: <23e2d1d2-0637-46fa-ba5c-002f292e62c7@oracle.com> Content-Type: text/plain; charset=UTF-8; format=flowed X-TM-AS-GCONF: 00 X-Proofpoint-GUID: EQAxMWBY6JNiizGh1jsDPYTJ8xEj_wvl X-Proofpoint-ORIG-GUID: EQAxMWBY6JNiizGh1jsDPYTJ8xEj_wvl Content-Transfer-Encoding: 8bit X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.293,Aquarius:18.0.1011,Hydra:6.0.650,FMLib:17.11.176.26 definitions=2024-04-23_12,2024-04-23_02,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 bulkscore=0 malwarescore=0 adultscore=0 mlxscore=0 suspectscore=0 phishscore=0 mlxlogscore=999 priorityscore=1501 impostorscore=0 clxscore=1015 spamscore=0 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2404010000 definitions=main-2404230037 X-Spam-Status: No, score=-10.2 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Am 23.04.2024 um 01:58 schrieb Indu Bhagat: > On 4/22/24 08:58, Jens Remus wrote: >> If an architecture uses both SFrame RA and FP tracking SFrame assumes >> that the RA offset is the 2nd offset and the FP offset is the 3rd offset >> following the SFrame FRE. An architecture does not need to store both on >> the stack. SFrame cannot represent a FP without RA on stack, since it >> cannot distinguish whether the 2nd offset is the RA or FP offset. >> >> Use an invalid SFrame FRE RA offset value of zero as dummy padding to >> represent the FP being saved on the stack when the RA is not saved on >> the stack. >> >> include/ >>     * sframe.h (SFRAME_FRE_RA_OFFSET_INVALID): New macro defining >>     the invalid RA offset value used to represent a dummy padding >>     offset. >> >> gas/ >>     * gen-sframe.c (get_fre_num_offsets): Accommodate for dummy >>     padding RA offset if FP without RA on stack. >>     (sframe_get_fre_offset_size): Likewise. >>     (output_sframe_row_entry): Write a dummy padding RA offset >>     if FP without RA needs to be represented. >> >> libsframe/ >>     * sframe-dump.c (dump_sframe_func_with_fres): Treat invalid RA >>     offsets as if they were undefined. Display them as "u*" to >>     distinguish them. >> >> Signed-off-by: Jens Remus >> --- >> >> Notes (jremus): >>      This patch eliminates 497 occurrences of the warning "skipping >> SFrame >>      FDE due to FP without RA on stack" for a build of glibc on s390x. >> For >>      libc.so this increases the number of FDEs by 166 and the number of >>      FREs by 861, while adding 337 dummy padding RA offsets. With a total >>      of 28157 offsets the dummy padding offsets account for ~1.20 % of >> the >>      offsets. > > While this increase seems small, it does look wasteful. > > An orthogonal question below... > >>      SFrame statistics without patch: >>          VALUE        TOTAL      MIN        MAX        AVG >>          FDEs:        3478       -          -          - >>          FREs/FDE:    14441      1          15         4 >>          Offsets/FDE: 28157      1          31         8 >>             8-bit:    0          0          0          0 >>            16-bit:    28157      1          31         8 >>            32-bit:    0          0          0          0 >>          Offsets/FRE: 28157      1          3          1 >>             8-bit:    -          0          0          0 >>            16-bit:    -          1          3          1 >>            32-bit:    -          0          0          0 >>      SFrame statistics with patch applied: >>          VALUE        TOTAL      MIN        MAX        AVG >>          FDEs:        3644       -          -          - >>          FREs/FDE:    15302      1          20         4 >>          Offsets/FDE: 29944      1          38         8 >>             8-bit:    0          0          0          0 >>            16-bit:    29944      1          38         8 >>            32-bit:    0          0          0          0 >>          Offsets/FRE: 29944      1          3          1 >>             8-bit:    -          0          0          0 >>            16-bit:    -          1          3          1 >>            32-bit:    -          0          0          0 >>          O_Padd/FDE:  337        -          -          0 >>             8-bit:    0 >>            16-bit:    337 >>            32-bit:    0 >>      Note that on s390x the offsets are at minimum 16-bits in size, >> due to >>      the mandatory CFA offset being at least 160. >> > > IIUC, all stack layouts supported in the ABI use the offset 160. Is that > right ? I am wondering if adjusting the stored offsets in the SFrame > section (by decrementing 160 from it) will work ? > > If yes, we could encode this constant in SFrame aux hdr bytes for s390x. Thank you for the hint! Using a constant adjustment of -160 on s390x for the CFA offset from CFA base register should work to allow for 8-bit offsets to be used. Aren't all tracked offsets (i.e. CFA, FP, and RA) signed anyway? Thus applying a constant adjustment should work in any case? Couldn't it simply be an architecture specific constant in the code to begin with? For example a new macro, which is only applied when defined? #define S390_SFRAME_CFA_OFFSET_ADJUSTMENT -160 #define SFRAME_CFA_OFFSET_ADJUSTMENT S390_SFRAME_CFA_OFFSET_ADJUSTMENT Implementing this in the SFrame auxiliary header would of course allow to implement this enhancement at a later stage and to change the adjustment value in the future, as the linker can then either reject or merge different adjustment values during link editing. I wonder whether it would make sense to store the FP register number in the SFrame auxiliary header for s390x as well. Register 11 is just a convention of the compilers and not defined by the ABI. That would enable us to choose a different register as frame pointer in the future. > >>   gas/gen-sframe.c        | 50 +++++++++++++++++++---------------------- >>   include/sframe.h        |  9 ++++++-- >>   libsframe/sframe-dump.c |  4 ++++ >>   3 files changed, 34 insertions(+), 29 deletions(-) >> >> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c >> index 4cc86eb6c815..990b08d87953 100644 >> --- a/gas/gen-sframe.c >> +++ b/gas/gen-sframe.c >> @@ -347,7 +347,9 @@ get_fre_num_offsets (struct sframe_row_entry >> *sframe_fre) >>       fre_num_offsets++; >>   #ifdef SFRAME_FRE_RA_TRACKING >>     if (sframe_ra_tracking_p () >> -      && sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK) >> +      && (sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK >> +      /* Accommodate for padding RA offset if FP without RA on >> stack.  */ >> +      || sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)) >>       fre_num_offsets++; >>   #endif >>     return fre_num_offsets; >> @@ -371,9 +373,14 @@ sframe_get_fre_offset_size (struct >> sframe_row_entry *sframe_fre) >>     if (sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK) >>       bp_offset_size = get_offset_size_in_bytes (sframe_fre->bp_offset); >>   #ifdef SFRAME_FRE_RA_TRACKING >> -  if (sframe_ra_tracking_p () >> -      && sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK) >> -    ra_offset_size = get_offset_size_in_bytes (sframe_fre->ra_offset); >> +  if (sframe_ra_tracking_p ()) >> +    { >> +      if (sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK) >> +    ra_offset_size = get_offset_size_in_bytes (sframe_fre->ra_offset); >> +      /* Accommodate for padding RA offset if FP without RA on >> stack.  */ >> +      else if (sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK) >> +    ra_offset_size = get_offset_size_in_bytes >> (SFRAME_FRE_RA_OFFSET_INVALID); >> +    } >>   #endif >>     /* Get the maximum size needed to represent the offsets.  */ >> @@ -537,11 +544,19 @@ output_sframe_row_entry (symbolS *fde_start_addr, >>     fre_write_offsets++; >>   #ifdef SFRAME_FRE_RA_TRACKING >> -  if (sframe_ra_tracking_p () >> -      && sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK) >> +  if (sframe_ra_tracking_p ()) >>       { >> -      fre_offset_func_map[idx].out_func (sframe_fre->ra_offset); >> -      fre_write_offsets++; >> +      if (sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK) >> +    { >> +      fre_offset_func_map[idx].out_func (sframe_fre->ra_offset); >> +      fre_write_offsets++; >> +    } >> +      /* Write padding RA offset if FP without RA on stack.  */ >> +      else if (sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK) >> +    { >> +      fre_offset_func_map[idx].out_func (SFRAME_FRE_RA_OFFSET_INVALID); >> +      fre_write_offsets++; >> +    } >>       } >>   #endif >>     if (sframe_fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK) >> @@ -1497,25 +1512,6 @@ sframe_do_fde (struct sframe_xlate_ctx *xlate_ctx, >>       = get_dw_fde_end_addrS (xlate_ctx->dw_fde); >>       } >> -#ifdef SFRAME_FRE_RA_TRACKING >> -  if (sframe_ra_tracking_p ()) >> -    { >> -      struct sframe_row_entry *fre; >> - >> -      /* Iterate over the scratchpad FREs and validate them.  */ >> -      for (fre = xlate_ctx->first_fre; fre; fre = fre->next) >> -    { >> -      /* SFrame format cannot represent FP on stack without RA on >> stack.  */ >> -      if (fre->ra_loc != SFRAME_FRE_ELEM_LOC_STACK >> -          && fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK) >> -        { >> -          as_warn (_("skipping SFrame FDE due to FP without RA on >> stack")); >> -          return SFRAME_XLATE_ERR_NOTREPRESENTED; >> -        } >> -    } >> -    } >> -#endif /* SFRAME_FRE_RA_TRACKING  */ >> - >>     return SFRAME_XLATE_OK; >>   } >> diff --git a/include/sframe.h b/include/sframe.h >> index 90bc92a32f84..d1a26875b3e2 100644 >> --- a/include/sframe.h >> +++ b/include/sframe.h >> @@ -237,6 +237,9 @@ typedef struct sframe_func_desc_entry >>      may or may not be tracked.  */ >>   #define SFRAME_FRE_FP_OFFSET_IDX    2 >> +/* Invalid RA offset.  Used as padding to represent FP without RA on >> stack.  */ >> +#define SFRAME_FRE_RA_OFFSET_INVALID 0 >> + >>   typedef struct sframe_fre_info >>   { >>     /* Information about >> @@ -288,9 +291,11 @@ typedef struct sframe_fre_info >>       offset1 (interpreted as CFA = BASE_REG + offset1) >>       if RA is being tracked >> -      offset2 (interpreted as RA = CFA + offset2) >> +      offset2 (interpreted as RA = CFA + offset2; an offset value of >> +           SFRAME_FRE_RA_OFFSET_INVALID indicates a dummy padding RA >> offset >> +           to represent FP without RA saved on stack) >>         if FP is being tracked >> -    offset3 (intrepreted as FP = CFA + offset2) >> +    offset3 (intrepreted as FP = CFA + offset3) > > I too noticed this typo recently and have a patch fixing this. > >>         fi >>       else >>         if FP is being tracked >> diff --git a/libsframe/sframe-dump.c b/libsframe/sframe-dump.c >> index 40ea531314ba..3ea4bc327efd 100644 >> --- a/libsframe/sframe-dump.c >> +++ b/libsframe/sframe-dump.c >> @@ -199,6 +199,10 @@ dump_sframe_func_with_fres (sframe_decoder_ctx >> *sfd_ctx, >>         if (sframe_decoder_get_fixed_ra_offset (sfd_ctx) >>         != SFRAME_CFA_FIXED_RA_INVALID) >>       strcpy (temp, "f"); >> +      /* If an ABI does track RA offset, e.g. AArch64 and S390, it >> can be a >> +     dummy as padding to represent FP without RA being saved on >> stack.  */ >> +      else if (err[2] == 0 && ra_offset == SFRAME_FRE_RA_OFFSET_INVALID) >> +    sprintf (temp, "u*"); >>         else if (err[2] == 0) >>       { >>         if (is_sframe_abi_arch_s390 (sfd_ctx) && (ra_offset & 1)) > Regards, Jens -- Jens Remus Linux on Z Development (D3303) and z/VSE Support +49-7031-16-1128 Office jremus@de.ibm.com IBM IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294 IBM Data Privacy Statement: https://www.ibm.com/privacy/