From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-00069f02.pphosted.com (mx0b-00069f02.pphosted.com [205.220.177.32]) by sourceware.org (Postfix) with ESMTPS id AD6E93851409 for ; Thu, 20 Oct 2022 12:34:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AD6E93851409 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=oracle.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=oracle.com Received: from pps.filterd (m0246632.ppops.net [127.0.0.1]) by mx0b-00069f02.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 29KCYD9Z031288 for ; Thu, 20 Oct 2022 12:34:47 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=from : to : cc : subject : references : date : in-reply-to : message-id : content-type : mime-version; s=corp-2022-7-12; bh=cAJ5ujB+NbeQtrkSzceW/yc90ugxQg9G7DsOS5feyWA=; b=Gr35lIbxd2DGibJlyCd35iDD066iUFe9WuGtNWJX2gNZV+ppz2C3bAddigHo2faS3OkB EBhPnnp4Fxw9PY2yFMbFJynGganWOjtCvHB186+/2RboiY5DZEh6e9ztk18SC3TY5D2s pU2MI6Jd2H73pXTwgOwXzX54VENdcl8DtY01AYIHkGl55y0yETDLUdAEiBhadMJiYRXy hNhwDcIjIOeifJyuOa625JUw4pfkQmlEOAysyq8yuNJghhcOQmLx65WeZhUOH8TB6nA4 ol/9VxaWv3BraGHdEEtZXynolOGM/CRK6ctL9i9wVlaTHhmYPPJdebgpZGuEahIcX4s5 TA== Received: from phxpaimrmta03.imrmtpd1.prodappphxaev1.oraclevcn.com (phxpaimrmta03.appoci.oracle.com [138.1.37.129]) by mx0b-00069f02.pphosted.com (PPS) with ESMTPS id 3k7mu059u7-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Thu, 20 Oct 2022 12:34:46 +0000 Received: from pps.filterd (phxpaimrmta03.imrmtpd1.prodappphxaev1.oraclevcn.com [127.0.0.1]) by phxpaimrmta03.imrmtpd1.prodappphxaev1.oraclevcn.com (8.17.1.5/8.17.1.5) with ESMTP id 29KAw2aj017052 for ; Thu, 20 Oct 2022 12:34:46 GMT Received: from nam02-bn1-obe.outbound.protection.outlook.com (mail-bn1nam07lp2041.outbound.protection.outlook.com [104.47.51.41]) by phxpaimrmta03.imrmtpd1.prodappphxaev1.oraclevcn.com (PPS) with ESMTPS id 3k8hu8qjph-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Thu, 20 Oct 2022 12:34:45 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Fg3iDv/hP0lkQLonjByB8NFYwAHUVbOawvFd3qUu6gqgZkdJKJWsNZv3ahpcanVSdJ0fcDtVykNmeDh4YBcwhc4l+k5HmJ9J80Ofq4JDNHXflZHE4IBrqdlwlQyumpqr0RM4F5/duPIg9OO+YJe6jRnexCr/bQuGaqEjBqdskpJK1A8heRC3I2x24Ntgvw1jqrUE1uOqtRTBYZHxkfEA/p2z38a2N5cDZTLuipsFN7RuxDyQCOUijKOBGIPfWfMSZhGWjQXAS45HIbzhiySHZzUQ5D+TDYSfkUQCqFQh1dKm8Q12UtQdNFage7ywUT5CPn2JuzjVlu/gYoRaQVUj1Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=cAJ5ujB+NbeQtrkSzceW/yc90ugxQg9G7DsOS5feyWA=; b=SnWp9IJlojzZxlg1G/Ozc0ttHrUm+UDZWjG/kjdVONnMt2oyXZp4iJSTxAy03sk33ArI7b/k3s6t1UA+CQGJUaOB8Gew80qv+tWvBk0dqixKKfdFpbt447gYsnM59OnyhAXXbnUyI0v9X/wez019ES54Mwn5yI5g8usxaX0m2K8KU67tQoBn3I1HT7S+VPa66gcXX57soqucNRy9uhxSYXLESJ+WP3+NDJJFWcoKp3y1DhyvGe8b9tlu8zwiqyJ9jNXjtnHrOW3aWaoUtFVt7keVP7MI9PDkXLFvm9NHA+R5z8RxtRY/E0kJe2t+gxpHlqAjT4bDCkAw++4KmhWTwQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=oracle.com; dmarc=pass action=none header.from=oracle.com; dkim=pass header.d=oracle.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.onmicrosoft.com; s=selector2-oracle-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=cAJ5ujB+NbeQtrkSzceW/yc90ugxQg9G7DsOS5feyWA=; b=WNk1MBzB/O5VqljGBO4mC0w/00g/0jYaqN3pKPQTz85Jm7rxP3QT1x1orAwZiyjhNqx0//fRqeCcSPzEeJAimu9qu5FoFNcBrqmy9k4Ggmz3Dv94AtnrNSFk1LVHLVF0dTq4lsn0dlBIOB5IWuOkOCYxqtt4dMDyR04XyX6wTrs= Received: from BYAPR10MB2888.namprd10.prod.outlook.com (2603:10b6:a03:88::32) by CY5PR10MB6120.namprd10.prod.outlook.com (2603:10b6:930:34::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5723.33; Thu, 20 Oct 2022 12:34:43 +0000 Received: from BYAPR10MB2888.namprd10.prod.outlook.com ([fe80::5095:b148:8def:1049]) by BYAPR10MB2888.namprd10.prod.outlook.com ([fe80::5095:b148:8def:1049%5]) with mapi id 15.20.5723.034; Thu, 20 Oct 2022 12:34:43 +0000 From: "Jose E. Marchesi" To: David Faust Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] bpf: add preserve_field_info builtin References: <20221019220638.13422-1-david.faust@oracle.com> Date: Thu, 20 Oct 2022 14:38:57 +0200 In-Reply-To: <20221019220638.13422-1-david.faust@oracle.com> (David Faust's message of "Wed, 19 Oct 2022 15:06:38 -0700") Message-ID: <8735bifz4u.fsf@oracle.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Content-Type: text/plain X-ClientProxiedBy: SG2P153CA0033.APCP153.PROD.OUTLOOK.COM (2603:1096:4:c7::20) To BYAPR10MB2888.namprd10.prod.outlook.com (2603:10b6:a03:88::32) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: BYAPR10MB2888:EE_|CY5PR10MB6120:EE_ X-MS-Office365-Filtering-Correlation-Id: bb8b1afc-2d53-4e66-3276-08dab29776a8 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 5alesCe6MGdvmf06hc34hfg+5AzpforXF34Z74AgnYXNtIIwIYlAXmcwBFhBFxnoH4vqkby+fzVmeTMdncg7RdeRmcSMFQK/295lMq+XmtvXnTjt0wTD/Rv0OSAMcKVCNibXRZ2WCaNpwJfAUjv1iOsQUtlYjlmlUQxKZfpv2c4GQYa3Cnmml6EO3bS7cL8pXgdZgubCiJJr/8V03aqcCsXK1FfwdE7a/oR8QqaN9nsBV6NHZFyuP8Ns/x0p6vXyz8O9QgIvdpc2294N8PeRySCvypwmIz8B/VZQbImy+0vjo2wLWU5TgPKWUVMzZmeDEY4VWDsfAtg/W4b5ePi688c713VT7q8/AqlKJE4kM2l4nM6PGt6nVIL1okhKYLY0NzGWWk20r/z+WGRn9DAb8+wUu9nrB/AGq2QVOsm4S0tPBY/TOafXKjVE7S+w+4T/52pqMza3RvQTPL4eosK+sSgycVJcy8rBqlgJnkvu0NjogcNYbeb0KkesMckKYoABTzPHreB6I/Kafty6ThbNuMd/StVe4nVJhkZyQQD6Lh3VvKJn1X8ZQGMQQZr8XqHIVe4IX5WbGbTsVKj7rcGCkDt7EUVRsQYrc/JdLEaCPXvvG014Hl7zli6KW6c3jtfgj7UZ+DK0JC42njpRN+YsgcbSSm49tnKENd2dlzNC32t/kNhfEW0u+7m0tHG6cfFvPomWSMtLaju+HP9gMNLqfQ== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BYAPR10MB2888.namprd10.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230022)(376002)(136003)(366004)(396003)(346002)(39860400002)(451199015)(38100700002)(36756003)(86362001)(30864003)(2906002)(6486002)(478600001)(41300700001)(66946007)(6506007)(8676002)(66476007)(66556008)(37006003)(6636002)(5660300002)(4326008)(6862004)(8936002)(316002)(6512007)(26005)(83380400001)(2616005)(6666004)(186003);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?l7P6kIb365vTe05Cay4dfWj+UfnOQFhcfnXJDGNsKWULf5qvSCN9H7ikITZ8?= =?us-ascii?Q?0a6+daC85/TCIKZKVoTu7wmq0O+Z7XfC4IfCoJapifpZK+gHDEacHTW+kOCE?= =?us-ascii?Q?RVK0I1GNMhvi9ORVmYEBWImHDt2YTfnDHXKqEIpeAaDejaecUPJruqZ7BKS5?= =?us-ascii?Q?sNnXX0QTAbGSTwBNV2edF3hPwPrYkoEBV41zRsCR5PhZIbG8rbpfsgdYyc/1?= =?us-ascii?Q?78LAD7UGL++dBxz/AE9p0l7bZ1dKXLxoV4hpBdKMTfAnlbewpXYkc4QyPFx1?= =?us-ascii?Q?m1/2nrColxk0IKgeR5nbPuem6BvsaeVklRUXPQIzpoHFJjN5E64GbFzZ8bD1?= =?us-ascii?Q?Urv7kG4BH+VPPZARHvOtK4NiiWU4rh+fZv3oBKrXqm2VFE+foJYZWOCtRQnb?= =?us-ascii?Q?jECtd47tHfkgVUvg/1PiLBW30UUo6zzuxeNz6VTl5LYQNTx9QEzrTlX2xewj?= =?us-ascii?Q?fJXqeDzcUJNNHofBgSoY/UUHOyjnRZlN9VmvYqAb+gwx3DBdzk4+dlAsgjWC?= =?us-ascii?Q?HOhLEbeGaxl4eiZm47XKDEwb2E2AP2BeTgLleoY8Qa80JsmLgQyUi9uVXv8l?= =?us-ascii?Q?2Y/d0wY2cVDfgANYERgPg8uSOxi4q7ORmzIYQ+y+9iNotCnW/RJE3fq33yr8?= =?us-ascii?Q?wr024dZ0iGQN22pPBRf7+c80nNMxhSH40kUxAuEUFGViBdkRTUeJMm8A7Skc?= =?us-ascii?Q?BV9xbfeyc55u0Fj5zs/lBw+qXSHAq7/+ZN4zQgqxdZhu/k2/e4eHuWUI8tjV?= =?us-ascii?Q?4NQ4kMhOrLYFs6zVlGQ7rGuAy2uC/wb/ZuNYDx6NqldKsN3S8WafdsU+dlPP?= =?us-ascii?Q?fX7tqcRM+KwaCcaZ5OIXzxTZgdTLqCJCo3HGP8/jOQ9ku2ee+6ERCRX/m7qy?= =?us-ascii?Q?SGu9Cwc0sFpExYvdLETxzVfXqoQHoDe+PArKjmzC+Sky9Kywo9QbDuerblhU?= =?us-ascii?Q?Fqv1KWOje7MjKaKpaOUr4vBvF/U46nmfTdyCOZVtpKGE4owVPIP/4zaXIab6?= =?us-ascii?Q?N9T94K5LEpXbs+RcGrJe55QgTZXlaBLMGPxBHGWEM3+kezD9cpksXYNng6IS?= =?us-ascii?Q?ojj7/gkxgsytSeC4thZ2BpZtNtMfEDNtQa46YK3xBhN9HCFwiNoUyc7sHL5+?= =?us-ascii?Q?p6wVDKcldFVo6tMRDrZW9JQr+ma9HQFF920OK/DE6dRiYS5Ht/Z4FAbqi6U6?= =?us-ascii?Q?eaUOSr2tsoBQINzTeJJ9UOm3w8H2L+uabEuu46ktcT9nFwH3e9jt+D5hbLeD?= =?us-ascii?Q?q2rUckVwa3vAMkaxsGx5AEmytQLqQ9NNTxRSmo+569I70Sfc4yJEnJYYTyjS?= =?us-ascii?Q?3z49lfTPi3rEF9Ksr50QX1qKWNjDpcV3XzhwmdJ1ngCM2ftjQzhTv+aIzdCS?= =?us-ascii?Q?fZ+3fQPggzTNz0WaoStOlneIYt8TvLuCFvCLYjpF67TJoktb+o8xa4bju1rU?= =?us-ascii?Q?bpngy+sXpCgfAsBBYKP3Dm8ASiVLhDf4mwiqhGb+maxEjWEB1G3e5hBUcWl1?= =?us-ascii?Q?8jbE1fIhUyRUEqy1e8XGR3ncjO//2v9nB8a9BuNrlC3WaZOCn/hcV2u9VA2K?= =?us-ascii?Q?GK6AlF5Q41zZwQS4XrAL1P5lJB/o1tT+9DcE58x+h3yA+HuMr0F5B75xUv6B?= =?us-ascii?Q?Qg=3D=3D?= X-OriginatorOrg: oracle.com X-MS-Exchange-CrossTenant-Network-Message-Id: bb8b1afc-2d53-4e66-3276-08dab29776a8 X-MS-Exchange-CrossTenant-AuthSource: BYAPR10MB2888.namprd10.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Oct 2022 12:34:43.1886 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: 7xIey5/c+VA+R4CcmTeWSSmPLQ9IlQ+L0DGc3N9PqzM5m/zy/4zGGopS29CmYvAuZK31d6g+IHt8jUrd9WIfx6gYFaOs1VA9UtqQVCBSrMM= X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR10MB6120 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1 definitions=2022-10-20_04,2022-10-20_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 mlxlogscore=999 malwarescore=0 spamscore=0 suspectscore=0 phishscore=0 adultscore=0 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2209130000 definitions=main-2210200074 X-Proofpoint-ORIG-GUID: Ue_nYQLbFJdEkw7ypN6iJ69XM_FgTWbD X-Proofpoint-GUID: Ue_nYQLbFJdEkw7ypN6iJ69XM_FgTWbD X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SOMETLD_ARE_BAD_TLD,RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,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: Hi David. Thanks for the patch. Please see a few comments below. > @@ -975,6 +978,161 @@ static tree bpf_core_compute (tree, vec *); > static int bpf_core_get_index (const tree); > static bool is_attr_preserve_access (tree); > > +static void > +maybe_make_core_relo (tree expr, enum btf_core_reloc_kind kind) This function is missing a comment explaining what it does. > +{ > + /* If we are not targetting BPF CO-RE, do not make a relocation. We > + might not be generating any debug info at all. */ > + if (!TARGET_BPF_CORE) > + return; > + > + auto_vec accessors; > + tree container = bpf_core_compute (expr, &accessors); > + > + /* Any valid use of the builtin must have at least one access. Otherwise, > + there is nothing to record and nothing to do. This is primarily a > + guard against optimizations leading to unexpected expressions in the > + argument of the builtin. For example, if the builtin is used to read > + a field of a structure which can be statically determined to hold a > + constant value, the argument to the builtin will be optimized to that > + constant. This is OK, and means the builtin call is superfluous. > + e.g. > + struct S foo; > + foo.a = 5; > + int x = __preserve_access_index (foo.a); > + ... do stuff with x > + 'foo.a' in the builtin argument will be optimized to '5' with -01+. > + This sequence does not warrant recording a CO-RE relocation. */ > + > + if (accessors.length () < 1) > + return; > + accessors.reverse (); > + > + rtx_code_label *label = gen_label_rtx (); > + LABEL_PRESERVE_P (label) = 1; > + emit_label (label); > + > + /* Determine what output section this relocation will apply to. > + If this function is associated with a section, use that. Otherwise, > + fall back on '.text'. */ > + const char * section_name; > + if (current_function_decl && DECL_SECTION_NAME (current_function_decl)) > + section_name = DECL_SECTION_NAME (current_function_decl); > + else > + section_name = ".text"; > + > + /* Add the CO-RE relocation information to the BTF container. */ > + bpf_core_reloc_add (TREE_TYPE (container), section_name, &accessors, label, > + kind); > +} > + > +/* Expand a call to __builtin_preserve_field_info by evaluating the requested > + information about SRC according to KIND, and return a tree holding > + the result. */ > + > +static tree > +bpf_core_field_info (tree src, enum btf_core_reloc_kind kind) > +{ > + unsigned int result; > + poly_int64 bitsize, bitpos; > + tree var_off; > + machine_mode mode; > + int unsignedp, reversep, volatilep; > + > + get_inner_reference (src, &bitsize, &bitpos, &var_off, &mode, &unsignedp, > + &reversep, &volatilep); Since the information returned by the builtin is always constant (positions, sizes) I think you will want to adjust the code for the eventuality of variable positioned fields and also variable sized fields. get_inner_reference sets var_off to a tree if the position of the field is variable. In these cases `bitpos' is relative to that position. Likewise, get_inner_reference sets `mode' is set to BLKmode and `bitsize' will be set to -1. I'm not sure what the built-in is supposed to do/return in these cases. I guess it makes sense to error out, but what does LLVM do? > + > + /* Note: Use DECL_BIT_FIELD_TYPE rather than DECL_BIT_FIELD here, because it > + remembers whether the field in question was originally declared as a > + bitfield, regardless of how it has been optimized. */ > + bool bitfieldp = (TREE_CODE (src) == COMPONENT_REF > + && DECL_BIT_FIELD_TYPE (TREE_OPERAND (src, 1))); > + > + unsigned int align = TYPE_ALIGN (TREE_TYPE (src)); > + if (TREE_CODE (src) == COMPONENT_REF) > + { > + tree field = TREE_OPERAND (src, 1); > + if (DECL_BIT_FIELD_TYPE (field)) > + align = TYPE_ALIGN (DECL_BIT_FIELD_TYPE (field)); > + else > + align = TYPE_ALIGN (TREE_TYPE (field)); > + } > + > + unsigned int start_bitpos = bitpos & ~(align - 1); > + unsigned int end_bitpos = start_bitpos + align; > + > + switch (kind) > + { > + case BPF_RELO_FIELD_BYTE_OFFSET: > + { > + if (bitfieldp) > + result = start_bitpos / 8; > + else > + result = bitpos / 8; > + } > + break; > + > + case BPF_RELO_FIELD_BYTE_SIZE: > + { > + if (bitfieldp) > + { > + /* To match LLVM behavior, byte size of bitfields is recorded as > + the full size of the base type. A 3-bit bitfield of type int is > + therefore recorded as having a byte size of 4 bytes. */ > + result = end_bitpos - start_bitpos; > + if (result & (result - 1)) > + error ("unsupported field expression"); > + result = result / 8; > + } > + else > + result = bitsize / 8; > + } > + break; > + > + case BPF_RELO_FIELD_EXISTS: > + /* The field always exists at compile time. */ > + result = 1; > + break; > + > + case BPF_RELO_FIELD_SIGNED: > + result = !unsignedp; > + break; > + > + case BPF_RELO_FIELD_LSHIFT_U64: > + case BPF_RELO_FIELD_RSHIFT_U64: > + { > + if (!bitfieldp) > + { > + if (bitsize > 64) > + error ("field size too large"); > + result = 64 - bitsize; > + break; > + } > + > + if (end_bitpos - start_bitpos > 64) > + error ("field size too large"); > + > + if (kind == BPF_RELO_FIELD_LSHIFT_U64) > + { > + if (TARGET_BIG_ENDIAN) > + result = bitpos + 64 - start_bitpos - align; > + else > + result = start_bitpos + 64 - bitpos - bitsize; > + } > + else /* RSHIFT_U64 */ > + result = 64 - bitsize; > + } > + break; > + > + default: > + error ("invalid argument to built-in function"); > + return error_mark_node; > + break; > + } > + > + return build_int_cst (unsigned_type_node, result); > +} > + > /* Expand a call to a BPF-specific built-in function that was set up > with bpf_init_builtins. */ > > @@ -1025,17 +1183,15 @@ bpf_expand_builtin (tree exp, rtx target ATTRIBUTE_UNUSED, > /* The result of the load is in R0. */ > return gen_rtx_REG (ops[0].mode, BPF_R0); > } > + > else if (code == -1) > { > - /* A resolved overloaded builtin, e.g. __bpf_preserve_access_index_si */ > + /* A resolved overloaded __builtin_preserve_access_index. */ > tree arg = CALL_EXPR_ARG (exp, 0); > > if (arg == NULL_TREE) > return NULL_RTX; > > - auto_vec accessors; > - tree container; > - > if (TREE_CODE (arg) == SSA_NAME) > { > gimple *def_stmt = SSA_NAME_DEF_STMT (arg); > @@ -1049,51 +1205,42 @@ bpf_expand_builtin (tree exp, rtx target ATTRIBUTE_UNUSED, > /* Avoid double-recording information if the argument is an access to > a struct/union marked __attribute__((preserve_access_index)). This > Will be handled by the attribute handling pass. */ > - if (is_attr_preserve_access (arg)) > - return expand_normal (arg); > - > - container = bpf_core_compute (arg, &accessors); > - > - /* Any valid use of the builtin must have at least one access. Otherwise, > - there is nothing to record and nothing to do. This is primarily a > - guard against optimizations leading to unexpected expressions in the > - argument of the builtin. For example, if the builtin is used to read > - a field of a structure which can be statically determined to hold a > - constant value, the argument to the builtin will be optimized to that > - constant. This is OK, and means the builtin call is superfluous. > - e.g. > - struct S foo; > - foo.a = 5; > - int x = __preserve_access_index (foo.a); > - ... do stuff with x > - 'foo.a' in the builtin argument will be optimized to '5' with -01+. > - This sequence does not warrant recording a CO-RE relocation. */ > - > - if (accessors.length () < 1) > - return expand_normal (arg); > - > - accessors.reverse (); > - > - container = TREE_TYPE (container); > - > - rtx_code_label *label = gen_label_rtx (); > - LABEL_PRESERVE_P (label) = 1; > - emit_label (label); > - > - /* Determine what output section this relocation will apply to. > - If this function is associated with a section, use that. Otherwise, > - fall back on '.text'. */ > - const char * section_name; > - if (current_function_decl && DECL_SECTION_NAME (current_function_decl)) > - section_name = DECL_SECTION_NAME (current_function_decl); > + if (!is_attr_preserve_access (arg)) > + maybe_make_core_relo (arg, BPF_RELO_FIELD_BYTE_OFFSET); > + > + return expand_normal (arg); > + } > + > + else if (code == -2) > + { > + /* A resolved overloaded __builtin_preserve_field_info. */ > + tree src = CALL_EXPR_ARG (exp, 0); > + tree kind_tree = CALL_EXPR_ARG (exp, 1); > + unsigned HOST_WIDE_INT kind_val; > + if (tree_fits_uhwi_p (kind_tree)) > + kind_val = tree_to_uhwi (kind_tree); > else > - section_name = ".text"; > + error ("invalid argument to built-in function"); > > - /* Add the CO-RE relocation information to the BTF container. */ > - bpf_core_reloc_add (container, section_name, &accessors, label); > + enum btf_core_reloc_kind kind = (enum btf_core_reloc_kind) kind_val; > > - return expand_normal (arg); > + if (TREE_CODE (src) == SSA_NAME) > + { > + gimple *def_stmt = SSA_NAME_DEF_STMT (src); > + if (is_gimple_assign (def_stmt)) > + src = gimple_assign_rhs1 (def_stmt); > + } > + if (TREE_CODE (src) == ADDR_EXPR) > + src = TREE_OPERAND (src, 0); > + > + tree result = bpf_core_field_info (src, kind); If I read this properly, for something like: __builtin_preserve_field_info (a = foo.bar + bar.baz, KIND) On one side CO-RE relocations are computed for both foo.bar and bar.baz (I see bpf_core_compute does that) as expected. But then the builtin returns information that can only apply to one access. Which one? > + > + if (result != error_mark_node) > + maybe_make_core_relo (src, kind); > + > + return expand_normal (result); > } > + > gcc_unreachable (); > } > > @@ -1259,17 +1406,29 @@ bpf_core_get_index (const tree node) > __builtin_preserve_access_index. */ > > static tree > -bpf_core_newdecl (tree type) > +bpf_core_newdecl (tree type, bool is_pai) > { > - tree rettype = build_function_type_list (type, type, NULL); > + tree rettype; > char name[80]; > - int len = snprintf (name, sizeof (name), "%s", "__builtin_pai_"); > + static unsigned long pai_count = 0; > + static unsigned long pfi_count = 0; > > - static unsigned long cnt = 0; > - len = snprintf (name + len, sizeof (name) - len, "%lu", cnt++); > + if (is_pai) > + { > + rettype = build_function_type_list (type, type, NULL); > + int len = snprintf (name, sizeof (name), "%s", "__builtin_pai_"); > + len = snprintf (name + len, sizeof (name) - len, "%lu", pai_count++); > + } > + else > + { > + rettype = build_function_type_list (unsigned_type_node, type, > + unsigned_type_node, NULL); > + int len = snprintf (name, sizeof (name), "%s", "__builtin_pfi_"); > + len = snprintf (name + len, sizeof (name) - len, "%lu", pfi_count++); > + } > > - return add_builtin_function_ext_scope (name, rettype, -1, BUILT_IN_MD, NULL, > - NULL_TREE); > + return add_builtin_function_ext_scope (name, rettype, is_pai ? -1 : -2, > + BUILT_IN_MD, NULL, NULL_TREE); > } > > /* Return whether EXPR could access some aggregate data structure that > @@ -1278,22 +1437,33 @@ bpf_core_newdecl (tree type) > static int > bpf_core_is_maybe_aggregate_access (tree expr) > { > - enum tree_code code = TREE_CODE (expr); > - if (code == COMPONENT_REF || code == ARRAY_REF) > - return 1; > - > - if (code == ADDR_EXPR) > + switch (TREE_CODE (expr)) > + { > + case COMPONENT_REF: > + case BIT_FIELD_REF: > + case ARRAY_REF: > + case ARRAY_RANGE_REF: > + return 1; > + case ADDR_EXPR: > + case NOP_EXPR: > return bpf_core_is_maybe_aggregate_access (TREE_OPERAND (expr, 0)); > - > + default:; > + } > return 0; > } > > +struct core_walk_data { > + location_t loc; > + tree arg; > +}; > + > /* Callback function used with walk_tree from bpf_resolve_overloaded_builtin. */ > > static tree > bpf_core_walk (tree *tp, int *walk_subtrees, void *data) > { > - location_t loc = *((location_t *) data); > + struct core_walk_data *dat = (struct core_walk_data *) data; > + bool is_pai = dat->arg == NULL_TREE; > > /* If this is a type, don't do anything. */ > if (TYPE_P (*tp)) > @@ -1302,10 +1472,18 @@ bpf_core_walk (tree *tp, int *walk_subtrees, void *data) > return NULL_TREE; > } > > + /* Build a new function call to a resolved builtin for the desired operation. > + If this is a preserve_field_info call, pass along the argument to the > + resolved builtin call. */ > if (bpf_core_is_maybe_aggregate_access (*tp)) > { > - tree newdecl = bpf_core_newdecl (TREE_TYPE (*tp)); > - tree newcall = build_call_expr_loc (loc, newdecl, 1, *tp); > + tree newdecl = bpf_core_newdecl (TREE_TYPE (*tp), is_pai); > + tree newcall; > + if (is_pai) > + newcall = build_call_expr_loc (dat->loc, newdecl, 1, *tp); > + else > + newcall = build_call_expr_loc (dat->loc, newdecl, 2, *tp, dat->arg); > + > *tp = newcall; > *walk_subtrees = 0; > } > @@ -1344,7 +1522,12 @@ bpf_small_register_classes_for_mode_p (machine_mode mode) > static tree > bpf_resolve_overloaded_builtin (location_t loc, tree fndecl, void *arglist) > { > - if (DECL_MD_FUNCTION_CODE (fndecl) != BPF_BUILTIN_PRESERVE_ACCESS_INDEX) > + bool is_pai = DECL_MD_FUNCTION_CODE (fndecl) > + == BPF_BUILTIN_PRESERVE_ACCESS_INDEX; > + bool is_pfi = DECL_MD_FUNCTION_CODE (fndecl) > + == BPF_BUILTIN_PRESERVE_FIELD_INFO; > + > + if (!is_pai && !is_pfi) > return NULL_TREE; > > /* We only expect one argument, but it may be an arbitrarily-complicated > @@ -1352,16 +1535,17 @@ bpf_resolve_overloaded_builtin (location_t loc, tree fndecl, void *arglist) > vec *params = static_cast *> (arglist); > unsigned n_params = params ? params->length() : 0; > > - if (n_params != 1) > + if ((is_pai && n_params != 1) || (is_pfi && n_params != 2)) > { > - error_at (loc, "expected exactly 1 argument"); > + error_at (loc, "wrong number of arguments"); > return NULL_TREE; > } > > tree param = (*params)[0]; > > - /* If not generating BPF_CORE information, the builtin does nothing. */ > - if (!TARGET_BPF_CORE) > + /* If not generating BPF_CORE information, preserve_access_index does nothing, > + and simply "resolves to" the argument. */ > + if (!TARGET_BPF_CORE && is_pai) > return param; > > /* Do remove_c_maybe_const_expr for the arg. > @@ -1387,7 +1571,11 @@ bpf_resolve_overloaded_builtin (location_t loc, tree fndecl, void *arglist) > This ensures that all the relevant information remains within the > expression trees the builtin finally gets. */ > > - walk_tree (¶m, bpf_core_walk, (void *) &loc, NULL); > + struct core_walk_data data; > + data.loc = loc; > + data.arg = is_pai ? NULL_TREE : (*params)[1]; > + > + walk_tree (¶m, bpf_core_walk, (void *) &data, NULL); > > return param; > } > @@ -1524,7 +1712,8 @@ handle_attr_preserve (function *fn) > emit_label (label); > > /* Add the CO-RE relocation information to the BTF container. */ > - bpf_core_reloc_add (container, section_name, &accessors, label); > + bpf_core_reloc_add (container, section_name, &accessors, label, > + BPF_RELO_FIELD_BYTE_OFFSET); > } > } > } > diff --git a/gcc/config/bpf/coreout.cc b/gcc/config/bpf/coreout.cc > index 8897a045ea1..9f71040846b 100644 > --- a/gcc/config/bpf/coreout.cc > +++ b/gcc/config/bpf/coreout.cc > @@ -152,7 +152,8 @@ static GTY (()) vec *bpf_core_sections; > > void > bpf_core_reloc_add (const tree type, const char * section_name, > - vec *accessors, rtx_code_label *label) > + vec *accessors, rtx_code_label *label, > + enum btf_core_reloc_kind kind) > { > char buf[40]; > unsigned int i, n = 0; > @@ -173,7 +174,7 @@ bpf_core_reloc_add (const tree type, const char * section_name, > > bpfcr->bpfcr_type = get_btf_id (ctf_lookup_tree_type (ctfc, type)); > bpfcr->bpfcr_insn_label = label; > - bpfcr->bpfcr_kind = BPF_RELO_FIELD_BYTE_OFFSET; > + bpfcr->bpfcr_kind = kind; > > /* Add the CO-RE reloc to the appropriate section. */ > bpf_core_section_ref sec; > diff --git a/gcc/config/bpf/coreout.h b/gcc/config/bpf/coreout.h > index 3c7bdfd8c2f..498853f6e00 100644 > --- a/gcc/config/bpf/coreout.h > +++ b/gcc/config/bpf/coreout.h > @@ -103,7 +103,7 @@ extern void btf_ext_init (void); > extern void btf_ext_output (void); > > extern void bpf_core_reloc_add (const tree, const char *, vec *, > - rtx_code_label *); > + rtx_code_label *, enum btf_core_reloc_kind); > extern int bpf_core_get_sou_member_index (ctf_container_ref, const tree); > > #ifdef __cplusplus > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi > index 04af0584d82..0d3bcb24ab9 100644 > --- a/gcc/doc/extend.texi > +++ b/gcc/doc/extend.texi > @@ -15745,6 +15745,27 @@ Load 32-bits from the @code{struct sk_buff} packet data pointed by the register > BPF Compile Once-Run Everywhere (CO-RE) support. Instruct GCC to generate CO-RE relocation records for any accesses to aggregate data structures (struct, union, array types) in @var{expr}. This builtin is otherwise transparent, the return value is whatever @var{expr} evaluates to. It is also overloaded: @var{expr} may be of any type (not necessarily a pointer), the return type is the same. Has no effect if @code{-mco-re} is not in effect (either specified or implied). > @end deftypefn > > +@deftypefn {Built-in Function} unsigned int __builtin_preserve_field_info (@var{expr}, unsigned int @var{kind}) > +BPF Compile Once-Run Everywhere (CO-RE) support. This builtin is used to > +extract information to aid in struct/union relocations. @var{expr} is > +an access to a field of a struct or union. Depending on @var{kind}, different > +information is returned to the program. A CO-RE relocation for the access in > +@var{expr} with kind @var{kind} is recorded if @code{-mco-re} is in effect. > + > +@var{kind} is one of: > +@smallexample > +enum > +@{ > + FIELD_BYTE_OFFSET = 0, > + FIELD_BYTE_SIZE, > + FIELD_EXISTENCE, > + FIELD_SIGNEDNESS, > + FIELD_LSHIFT_U64, > + FIELD_RSHIFT_U64, > +@}; > +@end smallexample > +@end deftypefn > + We need to document what all these returned values are. It may be more-or-less obvious, but it is better to be explicit IMO. It would also be good to document the fact the returned value is always known at compile-time.