From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 1JwMNX62gGBHRwAAWB0awg (envelope-from ) for ; Wed, 21 Apr 2021 19:34:22 -0400 Received: by simark.ca (Postfix, from userid 112) id CA81D1F104; Wed, 21 Apr 2021 19:34:22 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED, MAILING_LIST_MULTI,T_DKIM_INVALID,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id E99A31E783 for ; Wed, 21 Apr 2021 19:34:20 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6E1B83846035; Wed, 21 Apr 2021 23:34:20 +0000 (GMT) Received: from mail-ej1-x62a.google.com (mail-ej1-x62a.google.com [IPv6:2a00:1450:4864:20::62a]) by sourceware.org (Postfix) with ESMTPS id 888FF3857C74 for ; Wed, 21 Apr 2021 23:34:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 888FF3857C74 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=sifive.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=jimw@sifive.com Received: by mail-ej1-x62a.google.com with SMTP id sd23so57293863ejb.12 for ; Wed, 21 Apr 2021 16:34:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sifive.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=UwMsJnTivrLN7afBXqyz6B4mlo+zlhhtU13U5RoDlGA=; b=POimzRUzRAqM+CXYt1ISf995mnOEPv1h0RJ5Wc4U0exCETNIq/Yq6wHjjCXL3c8Dyi gtG3XtuHUnLRy+dyBTXIi7a/hg/dG67mbaE54dockR2dNCkkf2dg0m+JCoBtcLUByBCO 8A1MBhTbg4daDJzg/367NmgIjx7BwfFHWbPxniXSTIi+d5zvCR75guD5Towd1hJC/78w KYDSjvVsdbQMGqbq+NX7zCGmyoGbTpB+yXp6wOOq1OkVBFT2WCWVNFwksB2ztd62jpjH nKCGmQeF2vCLGrfQVucMj47PZ2100Dv/H34rvJPr+5fD6f+O8k6ATxy8HeHEUOqshQcf Ewpw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=UwMsJnTivrLN7afBXqyz6B4mlo+zlhhtU13U5RoDlGA=; b=GZ+xZuE/Qv+/9mOUGZVfkd8ULnX5aKtHTjdJ7WtAfPylP4uPDlpLBH/ctmrLH+DzkK UVI8WAUbtKYKO4kAsA2yyCOU4ah7OPp4N3JjsTmPRLLhzKmh6tXqrAHSaf8gXWVlUCOS mSiqvtj/FW65dClNotMpmT3bugc3f5Q8aQbYVpKUMUtytyk3YgfBd+eD5ELGQmU4LSUR DZvl4ykl2GEvigvLA5nxS1mYINbHcxKXfAt8Bcfhg65GEqjk7LIM1QXP8EKF5c9QEmX9 HbrRk4kmZlLQhCjctgFomY8ECaq3O3PsGBMptuaTzINMEbiK1GGtoLj0vHEl6ymd2Svn k6zg== X-Gm-Message-State: AOAM531xhesCZy3c/aCVk4iKxv9QjGnXU2gAW2xZlPv79+uKihtQ0zQe L9dxBDzPQyKdUTquKhyq5YEA8M40ohCG+IZtoPL7yA== X-Google-Smtp-Source: ABdhPJzQY4iqpGjcnFn1m1AUItqexLj8XwIAhp4f0qc1dKJRAKHLHXIz8im+06bDIvWIVU06B4prroeUlgVN/J8kB1M= X-Received: by 2002:a17:907:6e1:: with SMTP id yh1mr323004ejb.486.1619048052611; Wed, 21 Apr 2021 16:34:12 -0700 (PDT) MIME-Version: 1.0 References: <20210417175831.16413-1-jimw@sifive.com> <20210417175831.16413-7-jimw@sifive.com> In-Reply-To: From: Jim Wilson Date: Wed, 21 Apr 2021 16:34:01 -0700 Message-ID: Subject: Re: [PATCH 06/24] RISC-V: Add fp support. To: Jim Wilson , gdb-patches@sourceware.org, Monk Chiang Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" On Sun, Apr 18, 2021 at 9:08 PM Mike Frysinger wrote: > On 17 Apr 2021 10:58, Jim Wilson wrote: > > Add F and D instruction support. > > substance looks fine, just style nits > I fixed the formatting issues. tests ? > These are fairly well tested by the gcc testsuite. Sim tests would be good also, but I would prefer to handle that with a follow up patch when I have time to write the tests, or manage to find someone to write the tests for me. > > + switch (sim_fpu_is (&sfa)) > > + { > > + case SIM_FPU_IS_NINF: > > + cpu->regs[rd] = 1; > > + break; > > + case SIM_FPU_IS_NNUMBER: > > + cpu->regs[rd] = 1 << 1; > > i'm a little surprised all these bits don't have constants for them. but i > guess binutils wouldn't normally have broken out the FPU state into > headers ? > newlib and glibc have macros for them, which of course are named differently as no one ever thought about that, but we can't easily use either of those here, and the inconsistent naming could be a problem. binutils has no need to name these bits. There is no instruction that takes these bits as input, so they never appear directly in the assembly syntax. Of course, names could potentially be useful for people writing assembly code, but no one has asked for that yet, or volunteered to write it yet. We don't have an assembly language header file for people to include that could include these constants. We don't have a proper assembly language manual either. > > + case MATCH_FCVT_L_S: > > + TRACE_INSN (cpu, "fcvt.l.s %s, %s", > > + rd_name, frs1_name); > > + cpu->regs[rd] = (int64_t) cpu->fpregs[rs1].S[0]; > > + goto done; > > + case MATCH_FCVT_LU_S: > > + TRACE_INSN (cpu, "fcvt.lu.s %s, %s", > > + rd_name, frs1_name); > > + cpu->regs[rd] = (uint64_t) cpu->fpregs[rs1].S[0]; > > + goto done; > > + case MATCH_FCVT_S_L: > > + TRACE_INSN (cpu, "fcvt.s.l %s, %s", > > + frd_name, rs1_name); > > + cpu->fpregs[rd].S[0] = (float) ((int64_t) cpu->regs[rs1]); > > + goto done; > > + case MATCH_FCVT_S_LU: > > + TRACE_INSN (cpu, "fcvt.s.lu %s, %s", > > + frd_name, rs1_name); > > + cpu->fpregs[rd].S[0] = (float) cpu->regs[rs1]; > > these raw casts all feel ... wrong. are the semantics guaranteed to match > between whatever the host CPU is (e.g. x86_64) and the target (e.g. riscv) > ? > or it just seems to mostly work so we aren't going to squint too hard at > it ? > Yes, this is probably wrong. The other FP math is probably also wrong. RISC-V has canonical NaNs like ARM, but I don't see any obvious canonical NaN support in common/sim-fpu.c. I know that qemu handles this correctly. RISC-V has NaN boxing for FP values smaller than the FP register size, i.e. the upper bits must all be 1. I don't see the code doing this, but this would only matter for broken code so it is less important. Qemu still got this wrong last time I looked at it, but it has been a while since I looked at the qemu code. Maybe this stuff can be handled with a bug report and follow up patches? Jim