From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12174 invoked by alias); 21 Nov 2017 20:04:18 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 12154 invoked by uid 89); 21 Nov 2017 20:04:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,KB_WAM_FROM_NAME_SINGLEWORD,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=no version=3.3.2 spammy=HTo:U*tkoenig, yup X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-yw0-f176.google.com Received: from mail-yw0-f176.google.com (HELO mail-yw0-f176.google.com) (209.85.161.176) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 21 Nov 2017 20:04:16 +0000 Received: by mail-yw0-f176.google.com with SMTP id q37so6224101ywa.12; Tue, 21 Nov 2017 12:04:16 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=mZ6mkNThjjZyAVEHn9v3mO+NPcbMv+A1aT/xFPuxCsw=; b=kfa8B740iyHyWR8qdLDMAp6ALaNqXcN0YlZFadeGc6el3r6A2MnBlcBPqg2y7hAGcG y1ODVLkcSy6ot3IEqf6BM/7ktpZpgpoQ0J9Oq9+rha43Dq1m1uYJQCFuhP1dY7i7837W uT6Bo80vPdWG2ddNtkPgYFiI8UEejlP4tUQRY/ckLlofzS803yJ1ipRVJBeEDmp3k/b/ 8q1qYNp54/xQL1Q3cpep80DOa9xd6MFiCihsoNmtLeYZwrGQHd513slUjR4WiAjRBytv Qegx83QcCjo5r/HG+85dtKpDP0s+b9rt/02HfJ32C13oWBwXgY2KaJjLR6Zk4ile4Yeq IfXw== X-Gm-Message-State: AJaThX7t0EMhdUU0rin9dghklR7BApVgyXuF7EioufoDgVJmv0fxDJ30 xaHYTjWSIFmdGPvTzeQxmRweO3JGzbXXTEC6pwg= X-Google-Smtp-Source: AGs4zMZUhZvdsERAaGAi1dKLi7+VXVukehkSamF1kxc4tKGLqvqWCEhJcmxEkykZmbitpnAD3R7fcaiW1C9WMMKulnE= X-Received: by 10.129.174.76 with SMTP id g12mr12321365ywk.153.1511294655108; Tue, 21 Nov 2017 12:04:15 -0800 (PST) MIME-Version: 1.0 Received: by 10.129.163.146 with HTTP; Tue, 21 Nov 2017 12:04:14 -0800 (PST) In-Reply-To: <09d35ad1-6883-0372-7a1a-840c8c4b07f9@netcologne.de> References: <44c7b39b-d849-e31a-7175-80bf1a348908@netcologne.de> <03e85917-b506-0d8b-78b0-263c371cba6a@netcologne.de> <09d35ad1-6883-0372-7a1a-840c8c4b07f9@netcologne.de> From: Janne Blomqvist Date: Tue, 21 Nov 2017 20:47:00 -0000 Message-ID: Subject: Re: [patch, fortran] Implement maxloc and minloc for character To: Thomas Koenig Cc: "fortran@gcc.gnu.org" , gcc-patches Content-Type: text/plain; charset="UTF-8" X-SW-Source: 2017-11/txt/msg01953.txt.bz2 On Tue, Nov 21, 2017 at 9:50 PM, Thomas Koenig wrote: > Hi Janne, > > >>> So, any other comments about my patch? OK for trunk? >> >> >> - In many cases the copyright notice has "This file is part of the GNU >> Fortran 95 runtime library (libgfortran)." It's a while since we've >> called ourselves "GNU Fortran 95", so just remove the "95". > > > That's what I got for copying over an old version of maxloc > (when it still didn't have NAN handling) as a basis for my own > patch. This also meant that I had an old copyright notice. Fixed. Uh, it seems the patch you posted didn't actually fix this? >> - It seems in the library you're using int for string lengths? Please >> use gfc_charlen_type instead (in the frontend, gfc_charlen_type_node). >> (Most of the charlen->size_t patch is fixing up places where we're >> accidentally using int instead of gfc_charlen_type..). > > > Fixed. Not everywhere? At least zgrep "int len" p8.diff.gz turns up some cases? >> - Why are you using GFC_INTEGER_1 / GFC_INTEGER_4 to loop over the >> arrays rather than char/gfc_char4_t? Not sure if it makes any >> difference in practice, but it sure seems confusing.. > > > The reason has to do with evil m4 magic. I used a macro > from iparm.m4, atype_code. Changing m4 code should mostly > be restricted to those cases where it is _really_ necessary > (people, say, not without justification, that m4 is a write-only > langauge). Fair enough. :) >> - Not really related to your patch, but memcmp_char4 sure looks >> redundant. Isn't it the same as memcmp(a, b, size*4), in which case we >> could use optimized memcmp implementations? > > > Big/little endian issues. > > Consider the following short program: > > #include > #include > > char a[4] = { 1, 2, 3, 4}; > char b[4] = { 4, 3, 2, 1}; > > int main() > { > int i, j; > memcpy (&i, a, sizeof(i)); > memcpy (&j, b, sizeof(j)); > printf("memcmp : "); > if (memcmp (&i,&j,sizeof(i))) > printf("larger\n"); > else > printf("smaller\n"); > > printf("Direct comparison: "); > if (i > j) > printf("larger\n"); > else > printf("smaller\n"); > > return 0; > } > > On my x86_64 system, this prints > > memcmp : larger > > > Direct comparison: larger > > On a big-endian system, this prints > > memcmp : larger > Direct comparison: smaller Ooh, indeed. > However, I just learned about the __BYTE_ORDER__ macro. > We could use that (and in places where we currently have > a runtime check, for example in replacing the big_endian > global variable in libgfortran). But that is for another day. Yup. > So, attached is a new version of the patch. No update > on the ChangeLog. OK for trunk? Yup, just really fix the copyright and string length stuff first. Thanks! -- Janne Blomqvist