From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x335.google.com (mail-ot1-x335.google.com [IPv6:2607:f8b0:4864:20::335]) by sourceware.org (Postfix) with ESMTPS id 5460F3857C4C for ; Mon, 5 Apr 2021 07:37:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 5460F3857C4C Received: by mail-ot1-x335.google.com with SMTP id g8-20020a9d6c480000b02901b65ca2432cso10649974otq.3 for ; Mon, 05 Apr 2021 00:37:26 -0700 (PDT) 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:cc; bh=35Ih2tdnYWzah7AlpuglS/6xJPZX2FiIWlo+P3pui0E=; b=eIW+YwDEhclbOpFEDuB/5WzUaLyDBdoCNN65pBrtWlOWtToQHm8i6nfkdLNx94cfRK 2XONvoAtrb0d90NrMHijmsIu07pLdwx1U6Gtc1YFZhOiOT1adKaT28u9f/bhP/NsJ6F4 6Q3T9H2fFORWKjoT6D9FYcCzolcrDZOVXR4MvqBdmvZaw0NI1PUOrnzIwLbsnsQonhpe rXFrQtpLUtG3q//lgbpmoa44wuqZ+HRPFKT3VKv0elgJUF85XFhxCAWecm0Nhq6zqSmD OlavySS67nQy6BiOd7uM4J4Ku0LTSD/2yHuP/B856t+/I6ysWfCqrCkR75glk5Cp2b5I 1X4w== X-Gm-Message-State: AOAM530tlfUIudPpGLAjkyIBUjjg1YjbbCPOOhjQuEN3LTDmCC2SQD9G n7EbfxvB/+GhY/dtBYgn4KB+w+DzgdqdkLEJBuLgd6o/7aGQ3A== X-Google-Smtp-Source: ABdhPJz1iybHhUYJ0HH8FwnMp/86Gbb/Kn/O60BsZ9O8qfVKQpo1Jvc1DDx3JQZsA5Ztf4PC2hGDe7FFWWNQsdeABQw= X-Received: by 2002:a9d:6c52:: with SMTP id g18mr8212985otq.29.1617608245725; Mon, 05 Apr 2021 00:37:25 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: David Macek Date: Mon, 5 Apr 2021 09:37:14 +0200 Message-ID: Subject: Re: [PATCH] libc: Replace i386/sys/fenv.h symlink with an #include shim To: newlib@sourceware.org Cc: Corinna Vinschen Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: newlib@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Newlib mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 Apr 2021 07:37:27 -0000 Thank you. Below are my notes. I believe some are real issues, but most are novice questions, so be kind. :) > 3835c015d09d Add build mechanism to share common header files between machines I assume I only need to check Makefile.am (and acinclude.m4) here. It's good as far as I can tell, the order among other sources of header files seems reasonable. 1. I'm unsure about the indentation, the file already had been inconsistent, so I just note that the first hunk doesn't match its immediate surroundings. 2. I would expect shared_machine_dir to be introduced to configure.host in this patch as well, in the documentation and in the initialization block. > e8e95bf6436a configure.host: define shared ix86 and x86_64 directory 3. The globs in Makefile.am fail gracefully when they don't match, right? > 718f3955449f fenv: add missing declarations to x86 fenv.h Can't judge. > a0d06f6c50ed fenv: Move shared x86 sys/fenv.h from x86_64 to shared_x86 OK. > c06e30d3d961 fenv: move shared x86 fenv.c to libm/machine/shared_x86 OK. > 7803e212adcd fenv: drop Cygwin-specific implementation in favor of newlib code 4. A noticeable difference between cygwin's and newlib's fenv.c is use_sse being a run-time-initialized constant vs a (inline) function. Are we keeping the latter intentionally, or it doesn't matter? 5. Is it okay for autoload.cc and dcrt0.cc to still #include "fenv.h" instead of ? > 3b79d7e1c31f Cygwin: don't call _feinitialise from _dll_crt0 6. The date in the new fenv.c seems awfully in the past. 7. Why can we assume the FP environment is initialized? Is it because fesetenv() is called in autoload.cc, or is it just a guarantee from the OS? > Please fetch this change and reset your branch to this new state. > This should very much simplify reviewing the changes. 8. I noticed newlib/libm/fenv/fenv_stub.c still mentions symlinks. -- David Macek