From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x336.google.com (mail-ot1-x336.google.com [IPv6:2607:f8b0:4864:20::336]) by sourceware.org (Postfix) with ESMTPS id E059B3858C5E for ; Mon, 24 Jul 2023 15:53:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E059B3858C5E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-ot1-x336.google.com with SMTP id 46e09a7af769-6b9f46ec07aso2864105a34.1 for ; Mon, 24 Jul 2023 08:53:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1690214038; x=1690818838; h=mime-version:message-id:date:in-reply-to:subject:cc:to:from :user-agent:references:from:to:cc:subject:date:message-id:reply-to; bh=1f9jyo9asjlhNmbIA6eZzEaDFh6ZaOIMPIuVGz26tms=; b=XGItboDmwW97ngapgsgUNJyu6rmhkZxuXgOIIu7jD+PsMPklGmRPyc3RlHLgCeIN6m Fi0XuPYoVaYheDHxRVBA3z+4OtUz/C6gwSkK2/oa28rHHajRoLhU5EbgasfqSqwvByuI rjKrCt2A9X7Iws5fs0zw//sVEB2v34pbSYNkO6z1TbLF3/2/a/FPcqWh2n98+cQ+OEeg cXbAaPLEk0sWz7/SmFUhtCNOfMePIwbTaym5ItWrNdexHNsF5AL4/zgmZT5hOaAjjvxn Ra/mwqnbjeEJ1NdCigRO6plnu6bzU1tHfQrsOLZDhYNA+JxXpsgId+V1LSIzp84IvsxC Sfcg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690214038; x=1690818838; h=mime-version:message-id:date:in-reply-to:subject:cc:to:from :user-agent:references:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=1f9jyo9asjlhNmbIA6eZzEaDFh6ZaOIMPIuVGz26tms=; b=VrMq3HVEjbj6KqSszCsLAlDFu0RsGy3n2az2eOiNL+GqpjPy10KVJPhqlhGdu+jffu I7iVWKcypxMhL0HdQkPOYKCQLEFXlQVGxg4XVIcDOUHeMek9EMu4hQjBqI2BUOTvkQQQ mpWhsFbEmBKvKRyc6SAWpLxp+nbl7JaPICpIOqSL8ffos2r4OmKsEKU2aQLaGw7TOaQN dbROJBBP/mC5420KLni53B1yOhPmZliqiwee4wMomykGLo6k3ydlMrPU+/aqG5iz8mFO VXeSk6S4wlkv9j73GUxkeggP3i3K2AN3TntksFksdySaChhyQlunXr2DfvrfZxOVMd+o pWMg== X-Gm-Message-State: ABy/qLYvV+XNEoxmG+41S1RdOJ4jiF6gPkgN9c2jT759vSKuXM0iy6Mh jgEX2wYo+qUnBmA754ax7hCI/Q== X-Google-Smtp-Source: APBJJlGPgYdoJzR8zr2LRsJedO/t2ktQ0kU1R+WhOGAKmh1NBoXHvcL74bvB3WWuSW3b++ItZmdGqA== X-Received: by 2002:a05:6830:1611:b0:6b9:7b24:b353 with SMTP id g17-20020a056830161100b006b97b24b353mr5371048otr.6.1690214038254; Mon, 24 Jul 2023 08:53:58 -0700 (PDT) Received: from localhost ([2804:14d:7e39:8470:e1cb:a74f:236e:c03e]) by smtp.gmail.com with ESMTPSA id r2-20020a9d7cc2000000b006b8c277be12sm4032052otn.8.2023.07.24.08.53.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 24 Jul 2023 08:53:57 -0700 (PDT) References: <20230630134616.1238105-1-luis.machado@arm.com> <20230630134616.1238105-2-luis.machado@arm.com> User-agent: mu4e 1.10.5; emacs 28.2 From: Thiago Jung Bauermann To: Luis Machado Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v3 01/16] [gdb/aarch64] Fix register fetch/store order for native AArch64 Linux In-reply-to: <20230630134616.1238105-2-luis.machado@arm.com> Date: Mon, 24 Jul 2023 12:53:54 -0300 Message-ID: <87ila9jofh.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE 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: Hello Luis, I'm (slowly) going through your patches. I only found a few small nits so far. Luis Machado via Gdb-patches writes: > @@ -521,28 +522,28 @@ aarch64_fetch_registers (struct regcache *regcache, int regno) > if (tdep->has_tls ()) > fetch_tlsregs_from_thread (regcache); > } > + /* General purpose register? */ > else if (regno < AARCH64_V0_REGNUM) > fetch_gregs_from_thread (regcache); > - else if (tdep->has_sve ()) > + /* SVE register? */ > + else if (tdep->has_sve () && regno <= AARCH64_SVE_VG_REGNUM) > fetch_sveregs_from_thread (regcache); > - else > + /* FPSIMD register? */ > + else if (regno < AARCH64_FPCR_REGNUM) fetch_fpregs_from_thread reads AARCH64_FPCR_REGNUM, so shouldn't the if condition above use '<='? Ditto for aarch64_store_registers further below. > fetch_fpregs_from_thread (regcache); > - > - if (tdep->has_pauth ()) > - { > - if (regno == AARCH64_PAUTH_DMASK_REGNUM (tdep->pauth_reg_base) > - || regno == AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base)) > - fetch_pauth_masks_from_thread (regcache); > - } > - > - /* Fetch individual MTE registers. */ > - if (tdep->has_mte () > - && (regno == tdep->mte_reg_base)) > + /* PAuth register? */ > + else if (tdep->has_pauth () > + && (regno == AARCH64_PAUTH_DMASK_REGNUM (tdep->pauth_reg_base) > + || regno == AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base))) > + fetch_pauth_masks_from_thread (regcache); > + /* MTE register? */ > + else if (tdep->has_mte () > + && (regno == tdep->mte_reg_base)) > fetch_mteregs_from_thread (regcache); > - > - if (tdep->has_tls () > - && regno >= tdep->tls_regnum_base > - && regno < tdep->tls_regnum_base + tdep->tls_register_count) > + /* TLS register? */ > + else if (tdep->has_tls () > + && regno >= tdep->tls_regnum_base > + && regno < tdep->tls_regnum_base + tdep->tls_register_count) > fetch_tlsregs_from_thread (regcache); > } > > @@ -592,6 +593,7 @@ aarch64_store_registers (struct regcache *regcache, int regno) > aarch64_gdbarch_tdep *tdep > = gdbarch_tdep (regcache->arch ()); > > + /* Do we need to store all registers? */ > if (regno == -1) > { > store_gregs_to_thread (regcache); > @@ -606,22 +608,26 @@ aarch64_store_registers (struct regcache *regcache, int regno) > if (tdep->has_tls ()) > store_tlsregs_to_thread (regcache); > } > + /* General purpose register? */ > else if (regno < AARCH64_V0_REGNUM) > store_gregs_to_thread (regcache); > - else if (tdep->has_sve ()) > + /* SVE register? */ > + else if (tdep->has_sve () && regno <= AARCH64_SVE_VG_REGNUM) > store_sveregs_to_thread (regcache); > - else > + /* FPSIMD register? */ > + else if (regno < AARCH64_FPCR_REGNUM) Shouldn't it be "regno <= AARCH64_FPCR_REGNUM" here? > store_fpregs_to_thread (regcache); > - > - /* Store MTE registers. */ > - if (tdep->has_mte () > - && (regno == tdep->mte_reg_base)) > + /* MTE register? */ > + else if (tdep->has_mte () > + && (regno == tdep->mte_reg_base)) > store_mteregs_to_thread (regcache); > - > - if (tdep->has_tls () > - && regno >= tdep->tls_regnum_base > - && regno < tdep->tls_regnum_base + tdep->tls_register_count) > + /* TLS register? */ > + else if (tdep->has_tls () > + && regno >= tdep->tls_regnum_base > + && regno < tdep->tls_regnum_base + tdep->tls_register_count) > store_tlsregs_to_thread (regcache); > + > + /* PAuth registers are read-only. */ > } > > /* A version of the "store_registers" target_ops method used when running -- Thiago