From patchwork Tue Feb 27 12:49:23 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Botcazou X-Patchwork-Id: 86441 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id C888B385843E for ; Tue, 27 Feb 2024 12:50:10 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-wm1-x330.google.com (mail-wm1-x330.google.com [IPv6:2a00:1450:4864:20::330]) by sourceware.org (Postfix) with ESMTPS id DC0813858C36 for ; Tue, 27 Feb 2024 12:49:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DC0813858C36 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=adacore.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org DC0813858C36 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::330 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709038168; cv=none; b=YVyl1r6huFH6sm/2Z96pE0a6OUnX7uVuIW4ojSLVmBJQsCcr+Fb+2VD1te9kWQsXNLaOQS/UjUHVs9h75ocJRJHxAXBql1mFzGj8KrehElrTe0ecXGmoZstb7LqXIh0O+OGSGBXoD19Lbqnj8VcpyRtEwwo71oNvDbOJgeiJXok= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709038168; c=relaxed/simple; bh=o29x9mptjmNce9JUQv5ZszVvQkT4FGdj/6dP3j+/vw8=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=ZoJg2VpUhB1gUC+puMLvhdo9e05tO0LVSMYxeaMJ6H+jPcrGfxloJ+fXuslLhLqP+Jdao0e24E1N6jL8wEwUlDCK/EolR4yuA/AM5xF8hUWId0tmKISynsmEQmh6K2CG0Va03EwKDGLQKtSkNlS1DGyn3C6Ga/ZRQIicCOakqT8= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wm1-x330.google.com with SMTP id 5b1f17b1804b1-412a38e2adaso16593545e9.1 for ; Tue, 27 Feb 2024 04:49:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adacore.com; s=google; t=1709038164; x=1709642964; darn=gcc.gnu.org; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:from:to:cc:subject:date:message-id:reply-to; bh=aqBQMEmBsU9WZ48I9DWg0ukXo+ubXWN59VpcVYMMrjI=; b=bnlqISrHsUP9itn2IGmVx2HWwnJOsccNyprLYiVMSDt+y90nYJV0/wrjlPugHTOIFy caiyTPsn5RSvf/DFyGepmPPcrKhbvnTer6O7Rlr0495uBrV4wmgDfOL6F8Ob2oyDckxI A4RSEKoUSPT9RqF8Jbuk1D8TI2SnLOVSRt3nbjleofjMX7FTYZkIJmm3U7pcYhZPOsxX OCHsWhwP2C83MGkTAr9DkoMYt+7SZGsmDNI0bLYHHk/jIEvEjWhPQIFhocJBkczrxZW6 5xJSTLQ7x4KO6eBbVVr/CMp73RcmmSdpRX2cyDLbTgMovAIKxRppBcYA/ju0KRb1klCx D6ow== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709038164; x=1709642964; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=aqBQMEmBsU9WZ48I9DWg0ukXo+ubXWN59VpcVYMMrjI=; b=VVer9bwM6GdlSDEtO/N9FengqD2CQGjZsqIHWNJ7oWQY+45wRoyaoRwXRFhlzj2c/S gA8nEgkQNutYJUIMNvtoTzfiIs7cZpfsB+lsV1mYuGpi4eeQXWTQXudLobvzOb1mQ/zH gtw4ktQh+08DDaXnedLUVvlgcEBCM+vLxrCTMerjux5oeXzITgKCk8map75OhNwb65Sj AuKCmXAEthN37Kca+RqDAjtp9I/YHcP4pbWIsmT3ijvrf1tyjGkhuWZQnxbd1mY2cOj9 mYZvYwiVXPX1i5xOSJT/E+A5iyS4pXl28oZTIW2lX9pUJyoniLTWk9G1LWwv+360rZxn kt5A== X-Gm-Message-State: AOJu0YzjmfMtUTS63XgYT89ERxwwx3v0cl6HU1W7wRTveAzpQJOzgFuT NP/YPQvH/Rvdqmu38OJNFgBgnD0yesowGMLAznea7iVziS8jY/TdmvLF1MSuWH3Gv7MPIuCIYjs = X-Google-Smtp-Source: AGHT+IHFsMg6LaO99/DH5j+iYRhHzhepEdAdlzHWqXbVlqenzqdiBDJeBNKrILgD+pfA4xoz6mqb/Q== X-Received: by 2002:a05:600c:3c9d:b0:412:9736:d196 with SMTP id bg29-20020a05600c3c9d00b004129736d196mr9365178wmb.33.1709038164376; Tue, 27 Feb 2024 04:49:24 -0800 (PST) Received: from fomalhaut.localnet ([2a01:e0a:8d5:d990:e654:e8ff:fe8f:2ce6]) by smtp.gmail.com with ESMTPSA id p19-20020a05600c205300b00412945d2051sm11008221wmg.12.2024.02.27.04.49.23 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 27 Feb 2024 04:49:23 -0800 (PST) From: Eric Botcazou X-Google-Original-From: Eric Botcazou To: gcc-patches@gcc.gnu.org Subject: [PATCH] Fix internal error in GIMPLE DSE Date: Tue, 27 Feb 2024 13:49:23 +0100 Message-ID: <1958524.PYKUYFuaPT@fomalhaut> MIME-Version: 1.0 X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE, WEIRD_PORT autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Hi, this is a regression present on the mainline, 13 and 12 branches. For the attached Ada case, it's a tree checking failure on the mainline at -O: +===========================GNAT BUG DETECTED==============================+ | 14.0.1 20240226 (experimental) [master r14-9171-g4972f97a265] GCC error:| | tree check: expected tree that contains 'decl common' structure, | | have 'component_ref' in tree_could_trap_p, at tree-eh.cc:2733 | | Error detected around /home/eric/cvs/gcc/gcc/testsuite/gnat.dg/opt104.adb: Time is a 10-byte record and Packed_Rec.T is placed at bit-offset 65 because of the packing. so tree-ssa-dse.cc:setup_live_bytes_from_ref has computed a const_size of 88 from ref->offset of 65 and ref->max_size of 80. Then in tree-ssa-dse.cc:compute_trims: 411 int last_live = bitmap_last_set_bit (live); (gdb) next 412 if (ref->size.is_constant (&const_size)) (gdb) 414 int last_orig = (const_size / BITS_PER_UNIT) - 1; (gdb) 418 *trim_tail = last_orig - last_live; (gdb) call debug_bitmap (live) n_bits = 256, set = {0 1 2 3 4 5 6 7 8 9 10 } (gdb) p last_live $33 = 10 (gdb) p const_size $34 = 80 (gdb) p last_orig $35 = 9 (gdb) p *trim_tail $36 = -1 In other words, compute_trims is overlooking the alignment adjustments applied earlier by setup_live_bytes_from_ref. Moveover it reads: /* We use sbitmaps biased such that ref->offset is bit zero and the bitmap extends through ref->size. So we know that in the original bitmap bits 0..ref->size were true. We don't actually need the bitmap, just the REF to compute the trims. */ But setup_live_bytes_from_ref used ref->max_size instead of ref->size. It appears that all the callers of compute_trims assume that ref->offset is byte aligned and that the trimmed bytes are relative to ref->size, so the patch simply adds an early return if either condition is not fulfilled Tested on x86-64/Linux, OK for all the affected branches? 2024-02-27 Eric Botcazou * tree-ssa-dse.cc (compute_trims): Fix description. Return early if ref->offset is not byte aligned or ref->size is not known to be equal to ref->max_size. (maybe_trim_complex_store): Fix description. (maybe_trim_constructor_store): Likewise. (maybe_trim_partially_dead_store): Likewise. 2024-02-27 Eric Botcazou * gnat.dg/opt104.ads, gnat.dg/opt104.adb! New test. diff --git a/gcc/tree-ssa-dse.cc b/gcc/tree-ssa-dse.cc index 81b65125409..5869010287c 100644 --- a/gcc/tree-ssa-dse.cc +++ b/gcc/tree-ssa-dse.cc @@ -403,11 +403,11 @@ setup_live_bytes_from_ref (ao_ref *ref, sbitmap live_bytes) return false; } -/* Compute the number of elements that we can trim from the head and - tail of ORIG resulting in a bitmap that is a superset of LIVE. +/* Compute the number of stored bytes that we can trim from the head and + tail of REF. LIVE is the bitmap of stores to REF that are still live. - Store the number of elements trimmed from the head and tail in - TRIM_HEAD and TRIM_TAIL. + Store the number of bytes trimmed from the head and tail in TRIM_HEAD + and TRIM_TAIL respectively. STMT is the statement being trimmed and is used for debugging dump output only. */ @@ -416,10 +416,16 @@ static void compute_trims (ao_ref *ref, sbitmap live, int *trim_head, int *trim_tail, gimple *stmt) { - /* We use sbitmaps biased such that ref->offset is bit zero and the bitmap - extends through ref->size. So we know that in the original bitmap - bits 0..ref->size were true. We don't actually need the bitmap, just - the REF to compute the trims. */ + *trim_head = 0; + *trim_tail = 0; + + /* We use bitmaps biased such that ref->offset is contained in bit zero and + the bitmap extends through ref->max_size and we know that in the original + bitmap bits 0 .. ref->max_size were true. But we need to check that this + covers exactly the bytes of REF. */ + const unsigned int align = known_alignment (ref->offset); + if ((align && align < BITS_PER_UNIT) || !known_eq (ref->size, ref->max_size)) + return; /* Now identify how much, if any of the tail we can chop off. */ HOST_WIDE_INT const_size; @@ -444,8 +450,6 @@ compute_trims (ao_ref *ref, sbitmap live, int *trim_head, int *trim_tail, last_orig) <= 0) *trim_tail = 0; } - else - *trim_tail = 0; /* Identify how much, if any of the head we can chop off. */ int first_orig = 0; @@ -503,8 +507,7 @@ compute_trims (ao_ref *ref, sbitmap live, int *trim_head, int *trim_tail, } } - if ((*trim_head || *trim_tail) - && dump_file && (dump_flags & TDF_DETAILS)) + if ((*trim_head || *trim_tail) && dump_file && (dump_flags & TDF_DETAILS)) { fprintf (dump_file, " Trimming statement (head = %d, tail = %d): ", *trim_head, *trim_tail); @@ -513,9 +516,9 @@ compute_trims (ao_ref *ref, sbitmap live, int *trim_head, int *trim_tail, } } -/* STMT initializes an object from COMPLEX_CST where one or more of the - bytes written may be dead stores. REF is a representation of the - memory written. LIVE is the bitmap of stores that are actually live. +/* STMT initializes an object from COMPLEX_CST where one or more of the bytes + written may be dead stores. REF is a representation of the memory written. + LIVE is the bitmap of stores to REF that are still live. Attempt to rewrite STMT so that only the real or imaginary part of the object is actually stored. */ @@ -554,11 +557,10 @@ maybe_trim_complex_store (ao_ref *ref, sbitmap live, gimple *stmt) } /* STMT initializes an object using a CONSTRUCTOR where one or more of the - bytes written are dead stores. ORIG is the bitmap of bytes stored by - STMT. LIVE is the bitmap of stores that are actually live. + bytes written are dead stores. REF is a representation of the memory + written. LIVE is the bitmap of stores to REF that are still live. - Attempt to rewrite STMT so that only the real or imaginary part of - the object is actually stored. + Attempt to rewrite STMT so that only the live stores are performed. The most common case for getting here is a CONSTRUCTOR with no elements being used to zero initialize an object. We do not try to handle other @@ -780,9 +782,9 @@ maybe_trim_memstar_call (ao_ref *ref, sbitmap live, gimple *stmt) } } -/* STMT is a memory write where one or more bytes written are dead - stores. ORIG is the bitmap of bytes stored by STMT. LIVE is the - bitmap of stores that are actually live. +/* STMT is a memory write where one or more bytes written are dead stores. + REF is a representation of the memory written. LIVE is the bitmap of + stores to REF that are still live. Attempt to rewrite STMT so that it writes fewer memory locations. Right now we only support trimming at the start or end of the memory region.