From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id EIyiOWiZOGm1iSkAWB0awg (envelope-from ) for ; Tue, 09 Dec 2025 16:49:28 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1765316968; bh=5ruK9nndiPrP8akl+jQZtqb9sGLd6zWTt3Lj8sRNMgk=; h=Date:Subject:To:References:From:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=tEIqiXy8KkJ7+GA0ry+RLoK+x37nRlhve4nyN5ocOH2Q3YsgP5i2qGQzG04uUPQH6 CCwAQgxu1nPynZSRwVSH/ob5dUDNVHDpg6jCq8+FPJ460i0Qqif3rcw9kWiFVdcSGa yfo9EUvmnknHCG/Sl6TVz1OsQ6HgWR3u4epNirCg= Received: by simark.ca (Postfix, from userid 112) id DACB61E08D; Tue, 09 Dec 2025 16:49:28 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=bh7Jzc6x; dkim-atps=neutral Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 04E291E08D for ; Tue, 09 Dec 2025 16:49:28 -0500 (EST) Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id 860484BA2E1C for ; Tue, 9 Dec 2025 21:49:27 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 860484BA2E1C Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=bh7Jzc6x Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 7F9E14BA540C for ; Tue, 9 Dec 2025 21:48:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7F9E14BA540C Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 7F9E14BA540C Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=158.69.221.121 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1765316939; cv=none; b=Zrf0Q2ewFd2AapyV/AEJO02boPf8dS7pyfcbnCng3M+D+SDDx7SnnHnrCFr/+rEY3dJbSqwzX2vLoLXAEObS2c/HUTmz1AFpgz1yAJJbC57QijaS9KWzn3OmyXEKDPO19zKPMLM/2bS65Oah0w1ci+CWQ01Wj/8b1isoXlPekbE= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1765316939; c=relaxed/simple; bh=5ruK9nndiPrP8akl+jQZtqb9sGLd6zWTt3Lj8sRNMgk=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=CzFQ5pBpi3LO3WWnDeSKYFo/CpFFQxB7Yz5zkLvw3Xmt194NOs6BFpocvlcj+LzW3VVwVmUEcg5NkCXmCBfCN32fsLrG7rdIX4scglUoVnyCcURIjCrhgT+4vVZcK6s4SAYwWvWc3VAEO3llnKr0qqsKkaWehzkYPu6BagHz+As= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7F9E14BA540C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1765316939; bh=5ruK9nndiPrP8akl+jQZtqb9sGLd6zWTt3Lj8sRNMgk=; h=Date:Subject:To:References:From:In-Reply-To:From; b=bh7Jzc6x1DrRE4E5JoQvRHcStgTekBii+gvEApUNzQVYm8xkObp/Z7GSrfYA4TGh+ Vw7efT8lPOxXtUcVnFfbSKMfpyzsmvWKnn29euozDzDu9j3wRQrSPPap7i8gCAuv1y eebE/hFsi3yU97lvTVA3YTFkxgeZfun0EJdq+FFc= Received: by simark.ca (Postfix) id F16471E08D; Tue, 09 Dec 2025 16:48:58 -0500 (EST) Message-ID: <9399c753-fb5d-45ca-90a3-81f48b3b3077@simark.ca> Date: Tue, 9 Dec 2025 16:48:58 -0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 06/44] gdb, arch, intelgt: add intelgt arch definitions To: Tankut Baris Aktemur , gdb-patches@sourceware.org, Markus Metzger References: <20250801-upstream-intelgt-mvp-v3-0-59ce0f87075b@intel.com> <20250801-upstream-intelgt-mvp-v3-6-59ce0f87075b@intel.com> Content-Language: fr From: Simon Marchi In-Reply-To: <20250801-upstream-intelgt-mvp-v3-6-59ce0f87075b@intel.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org On 8/1/25 5:37 AM, Tankut Baris Aktemur wrote: > From: Markus Metzger > > Provide Intel GT architecture-specific definitions that can be used by > both the low target at the server side and tdep at the GDB side. > > Other than, for example, IA, Intel GT does not have a dedicated > breakpoint instruction. Instead, it has a breakpoint bit in each > instruction. We define arch methods for dealing with instruction > breakpoint bits. > > Co-authored-by: Tankut Baris Aktemur > Co-authored-by: Mihails Strasuns > Co-authored-by: Natalia Saiapova > Reviewed-By: Thiago Jung Bauermann This LGTM, see nits below. Approved-By: Simon Marchi > +#include "gdbsupport/tdesc.h" > +#include > +#include clangd tells me that all these includes are unused. > + > +namespace intelgt { Good idea to use a namespace here, we don't do that enough in GDB. > + > +/* Various arch constants. */ > + > +enum breakpoint_kind > +{ > + BP_INSTRUCTION = 1, > +}; enum class, for new stuff? You can get rid of the prefix then. > + > +/* The length of a full and compact IntelGT instruction in bytes. */ > + > +constexpr int MAX_INST_LENGTH = 16; > +constexpr int COMPACT_INST_LENGTH = 8; > + > +/* Feature names. > + > + They correspond to register sets defined in zet_intel_gpu_debug.h. We > + declare feature names in the order used in that header. > + > + The SBA register set consists of a set of base registers in the order > + defined in that header file. > + > + Not all registers have DWARF numbers. See DWARF_REGSETS below for a > + list of features that do. */ > +constexpr const char *FEATURE_GRF = "org.gnu.gdb.intelgt.grf"; > +constexpr const char *FEATURE_ADDR = "org.gnu.gdb.intelgt.addr"; > +constexpr const char *FEATURE_FLAG = "org.gnu.gdb.intelgt.flag"; > +constexpr const char *FEATURE_CE = "org.gnu.gdb.intelgt.ce"; > +constexpr const char *FEATURE_SR = "org.gnu.gdb.intelgt.sr"; > +constexpr const char *FEATURE_CR = "org.gnu.gdb.intelgt.cr"; > +constexpr const char *FEATURE_TDR = "org.gnu.gdb.intelgt.tdr"; > +constexpr const char *FEATURE_ACC = "org.gnu.gdb.intelgt.acc"; > +constexpr const char *FEATURE_MME = "org.gnu.gdb.intelgt.mme"; > +constexpr const char *FEATURE_SP = "org.gnu.gdb.intelgt.sp"; > +constexpr const char *FEATURE_SBA = "org.gnu.gdb.intelgt.sba"; > +constexpr const char *FEATURE_DBG = "org.gnu.gdb.intelgt.dbg"; > +constexpr const char *FEATURE_FC = "org.gnu.gdb.intelgt.fc"; > +constexpr const char *FEATURE_DEBUGGER = "org.gnu.gdb.intelgt.debugger"; > + > +/* Register sets/groups needed for DWARF mapping. Used for > + declaring static arrays for various mapping tables. */ > + > +enum dwarf_regsets : int > +{ > + REGSET_SBA = 0, > + REGSET_GRF, > + REGSET_ADDR, > + REGSET_FLAG, > + REGSET_ACC, > + REGSET_MME, > + REGSET_COUNT > +}; > + > +/* Map of dwarf_regset values to the target description > + feature names. */ > + > +constexpr const char *DWARF_REGSET_FEATURES[REGSET_COUNT] = { > + FEATURE_SBA, > + FEATURE_GRF, > + FEATURE_ADDR, > + FEATURE_FLAG, > + FEATURE_ACC, > + FEATURE_MME > +}; > + > +/* The encoding for XE version enumerates follows this pattern, which is enumerates -> enumerators? > + aligned with the IGA encoding. */ What is IGA? Around here it's a grocery store (https://www.iga.net). :) > + > +#define XE_VERSION(MAJ, MIN) (((MAJ) << 24) | (MIN)) With my modern C++ hat on: I am wondering if this can be a (constexpr?) function, just because. > +/* Supported GDB XE platforms. */ > + > +enum xe_version > +{ > + XE_INVALID = 0, > + XE_HP = XE_VERSION (1, 1), > + XE_HPG = XE_VERSION (1, 2), > + XE_HPC = XE_VERSION (1, 4), > + XE2 = XE_VERSION (2, 0), > + XE3 = XE_VERSION (3, 0), > +}; > + > +/* Helper function to translate the device id to a device version. */ > + > +extern xe_version get_xe_version (uint32_t device_id); I would remove this "extern" (and all the unnecessary uses of "extern" in the project actually). Simon