From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x643.google.com (mail-ej1-x643.google.com [IPv6:2a00:1450:4864:20::643]) by sourceware.org (Postfix) with ESMTPS id C9BC33861851 for ; Tue, 28 Jul 2020 06:56:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C9BC33861851 Received: by mail-ej1-x643.google.com with SMTP id y10so19573546eje.1 for ; Mon, 27 Jul 2020 23:56:09 -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=eVD5DEF704HkVPQbSLZ5H2jBzRt6NQWCrwgqxUoPVHk=; b=eodsbPAsWZESq+Pst4wf5bbkBSQz8HL2lcfuf7I0ow2OPDZWH8ziyiOMD1BQ4P3A8n cN7DIYueaCDNOQ8I1icYQG4fzxOJGoioNEemvjZNFZXNe6V+do0DUum9fuZpAZHeUoIK BBQqDvUbWf90IuAbnPIMtGw0eFQI6sc0Kcdylbf5toZ6nG5KqwlzdNRZFSt23Zt7eJ0r Wv+Bdlc7ouaofjp2rGsflHM4CXber5Ov0TMfUbcpMLpzyV02RspgR7/y3qR7/zqYZRs6 qvMN/FjKAVIRAo50w4vQML+fKnyDZh62tS8RBsz9fBnVQj91qxahQBuVlTAueRK+53zO Pl9g== X-Gm-Message-State: AOAM530s9m1SUUUGk6ZR+izIw8Q8ShsgQIrS7JwXiAeulmQzVrU1VLR4 Qfd7FVMg1tweYrDG29q64s19PsDZvijQhLUQ8Ck= X-Google-Smtp-Source: ABdhPJztu0zmPlB+SkxXp4XadwnTvqgarY35g6qPoM5s8bLmwkHEQusKagXS0NJ3f2Kt/Zsl/m+Pj15AtNE8guqfpps= X-Received: by 2002:a17:907:b0b:: with SMTP id h11mr5687841ejl.371.1595919368784; Mon, 27 Jul 2020 23:56:08 -0700 (PDT) MIME-Version: 1.0 References: <20200722151450.1540130-1-stefansf@linux.ibm.com> <20200727142019.GA901349@localhost.localdomain> In-Reply-To: <20200727142019.GA901349@localhost.localdomain> From: Richard Biener Date: Tue, 28 Jul 2020 08:55:57 +0200 Message-ID: Subject: Re: [PATCH] [RFC] vect: Fix infinite loop while determining peeling amount To: Stefan Schulze Frielinghaus Cc: Richard Biener via Gcc-patches , Richard Sandiford Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, KAM_SHORT, 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: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Jul 2020 06:56:11 -0000 On Mon, Jul 27, 2020 at 4:20 PM Stefan Schulze Frielinghaus wrote: > > On Mon, Jul 27, 2020 at 12:29:11PM +0200, Richard Biener wrote: > > On Mon, Jul 27, 2020 at 11:45 AM Richard Sandiford > > wrote: > > > > > > Richard Biener writes: > > > > On Mon, Jul 27, 2020 at 11:09 AM Richard Sandiford > > > > wrote: > > > >> > > > >> Richard Biener via Gcc-patches writes: > > > >> > On Wed, Jul 22, 2020 at 5:18 PM Stefan Schulze Frielinghaus via > > > >> > Gcc-patches wrote: > > > >> >> > > > >> >> This is a follow up to commit 5c9669a0e6c respectively discussion > > > >> >> https://gcc.gnu.org/pipermail/gcc-patches/2020-June/549132.html > > > >> >> > > > >> >> In case that an alignment constraint is less than the size of a > > > >> >> corresponding scalar type, ensure that we advance at least by one > > > >> >> iteration. For example, on s390x we have for a long double an alignment > > > >> >> constraint of 8 bytes whereas the size is 16 bytes. Therefore, > > > >> >> TARGET_ALIGN / DR_SIZE equals zero resulting in an infinite loop which > > > >> >> can be reproduced by the following MWE: > > > >> > > > > >> > But we guard this case with vector_alignment_reachable_p, so we shouldn't > > > >> > have ended up here and the patch looks bogus. > > > >> > > > >> The above sounds like it ought to count as reachable alignment though. > > > >> If a type requires a lower alignment than its size, then that's even > > > >> more easily reachable than a type that requires the same alignment as > > > >> the size. I guess at one extreme, a target alignment of 1 is always > > > >> reachable. > > > > > > > > Well, if the element alignment is 8 but its size is 16 then when presumably > > > > the desired vector alignment is a multiple of 16 we can never reach it. > > > > Isn't this the case here? > > > > > > If the desired vector alignment (TARGET_ALIGN) is a multiple of 16 then > > > TARGET_ALIGN / DR_SIZE will be nonzero and the problem the patch is > > > fixing wouldn't occur. I agree that we might never be able to reach > > > that alignment if the pointer starts out misaligned by 8 bytes. > > > > > > But I think that's why it makes sense for the target to only ask > > > for 8-byte alignment for vectors too, if it can cope with it. 8-byte > > > alignment should always be achievable if the scalars are ABI-aligned. > > > And if the target does ask for only 8-byte alignment, TARGET_ALIGN / > > > DR_SIZE would be zero and the loop would never progress, which is the > > > problem that the patch is fixing. > > > > > > It would even make sense for the target to ask for 1-byte alignment, > > > if the target doesn't care about alignment at all. > > > > Hmm, OK. Guess I still think we should detect this somewhere upward > > and avoid this peeling compute at all. Somehow. > > I've been playing around with another solution which works for me by > changing vector_alignment_reachable_p to return also false if the > alignment requirements are already satisfied, i.e., by adding: > > if (known_alignment_for_access_p (dr_info) && aligned_access_p (dr_info)) > return false; That sounds wrong, instead ... > Though, I'm not entirely sure whether this makes it better or not. > Strictly speaking if the alignment was reachable before peeling, then > reaching alignment with peeling is also possible but probably not what > was intended. So I guess returning false in this case is sensible. Any > comments? ... why is the DR considered for peeling at all? If it is already aligned there's no point to do that. If we want to align another DR then the loop you fix should run on that DRs align/size, no? Richard. > Thanks, > Stefan > > > > > Richard. > > > > > Thanks, > > > Richard