From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x230.google.com (mail-lj1-x230.google.com [IPv6:2a00:1450:4864:20::230]) by sourceware.org (Postfix) with ESMTPS id 410163858D28; Wed, 30 Nov 2022 07:53:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 410163858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lj1-x230.google.com with SMTP id l8so19923560ljh.13; Tue, 29 Nov 2022 23:53:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=he7rldmaR7aYhEnl4WWePqVuWpY2BiMWl4m9Sws3Uqk=; b=hPycXoDN2hX5ORgk5QkWKH7qYBjuqra3aGSkdgq/ucuW32Hvi2sk0HJdqMEP9KiOhh lJLPoda5esFNMftZX+ekyzj2nWqWpQ/vvGuXcC2U/qIJRc4bmkyGSQao6jcgAK4bOHWV 9stbXUuxI8CDRzJJxSa9r/E42Mq0cKrfn15ewRa+wqMHLXO0ypaV539ekgiheai2Ph1G tVzEIzPgKTdNdas/vXdeAu0dBMV8JMwKM/wWHkIJVuvJACr2mbO9ItvBp8TNsnuyCUlX 35YKiPjXI9grJchzBaxW8+rgDP8eg1cbHTg94zB4jTsrPG0npRxj5qaC9P98q4RwsH6d pBgA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=he7rldmaR7aYhEnl4WWePqVuWpY2BiMWl4m9Sws3Uqk=; b=cAn0tFytFI4fx5NTzmBst70UPXOrB06wH+n60LbcLL09n4iIH5bz7VfrzSeGEUOfRB NZcAAoCR2uqbi1M9BmHCZML8BzM1fkSmWSy8m0IxdwEfVqaZ6o0ldXbQjb66M+PDHpeU NffMAaCcf15dcYJQN/oQsk8WsckUh4g8obTcefNwb5y1xklqc6Hmk27cxkWcnSUaqrfn 0yu2mFEwFoiEGA8DF8eB9Qcp0zYW/zhkVWXBZMtEi1pDs25cM0YNkw4ATvMFl6THCvhY pEwZtSQVVh79jGz7Xq63yPpUugqNbHgZAamiGDuvvOzIQWNApdpziyw6SeiiJVZCOKeO MCqg== X-Gm-Message-State: ANoB5plB/zLIV3QaCiqjp7rWGpghdVYrlCHVGB35nYa2bqGJKjL2OWiA Qp0+MJA089WnNfNSRSal4FUCz7TnhduaT+839Ec= X-Google-Smtp-Source: AA0mqf714wZ/A+31j0rgKHk+d3U1GYApUyeocirwquaclEivSRAsXN5TNBnSyrVNZvUtvi1NquOQnBgve51s35dx7wE= X-Received: by 2002:a05:651c:19ab:b0:276:66a4:47c3 with SMTP id bx43-20020a05651c19ab00b0027666a447c3mr13307112ljb.49.1669794815853; Tue, 29 Nov 2022 23:53:35 -0800 (PST) MIME-Version: 1.0 References: <20221026081811.602573-1-arthur.cohen@embecosm.com> <20221026081811.602573-39-arthur.cohen@embecosm.com> <45ac1002-dcb6-6dad-a743-cd68c09c3cdd@embecosm.com> In-Reply-To: <45ac1002-dcb6-6dad-a743-cd68c09c3cdd@embecosm.com> From: Richard Biener Date: Wed, 30 Nov 2022 08:53:23 +0100 Message-ID: Subject: Re: [PATCH Rust front-end v3 38/46] gccrs: Add HIR to GCC GENERIC lowering entry point To: Arthur Cohen Cc: gcc-patches@gcc.gnu.org, gcc-rust@gcc.gnu.org, 90.abbasfaisal@gmail.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-0.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Tue, Nov 29, 2022 at 7:10 PM Arthur Cohen wrote: > > Hi Richard, > > (...) > > >>>> + > >>>> + unsigned HOST_WIDE_INT ltype_length > >>>> + = wi::ext (wi::to_offset (TYPE_MAX_VALUE (ltype_domain)) > >>>> + - wi::to_offset (TYPE_MIN_VALUE (ltype_domain)) + 1, > >>> > >>> TYPE_MIN_VALUE is not checked to be constant, also the correct > >>> check would be to use TREE_CODE (..) == INTEGER_CST, in > >>> the GCC middle-end an expression '1 + 2' (a PLUS_EXPR) would > >>> be TREE_CONSTANT but wi::to_offset would ICE. > >>> > >>>> + TYPE_PRECISION (TREE_TYPE (ltype_domain)), > >>>> + TYPE_SIGN (TREE_TYPE (ltype_domain))) > >>>> + .to_uhwi (); > >>> > >>> .to_uhwi will just truncate if the value doesn't fit, the same result as > >>> above is achieved with > >>> > >>> unsigned HOST_WIDE_INT ltype_length > >>> = TREE_INT_CST_LOW (TYPE_MAX_VALUE (..)) > >>> - TREE_INT_CST_LOW (TYPE_MIN_VALUE (...)) + 1; > >>> > >>> so it appears you wanted to be "more correct" here (but if I see > >>> correctly you fail on that attempt)? > >>> > > I've made the changes you proposed and noticed failure on our 32-bit CI. > > I've had a look at the values in detail, and it seems that truncating > was the expected behavior. > > On our 64 bit CI, with a testcase containing an array of zero elements, > we get the following values: > > TREE_INT_CST_LOW(TYPE_MAX_VALUE(...)) = 18446744073709551615; > TREE_INT_CST_LOW(TYPE_MIN_VALUE(...)) = 0; > > Adding 1 to the result of the substraction results in an overflow, > wrapping back to zero. > > With the -m32 flag, we get the following values: > > TREE_INT_CST_LOW(TYPE_MAX_VALUE(...)) = 4294967295; > TREE_INT_CST_LOW(TYPE_MIN_VALUE(...)) = 0; > > The addition of 1 does not overflow the unsigned HOST_WIDE_INT type and > we end up with 4294967296 as the length of our array. > > I am not sure on how to fix this behavior, and whether or not it is the > expected one, nor am I familiar enough with the tree API to reproduce > the original behavior. Any input is welcome. > > In the meantime, I'll revert those changes and probably keep the > existing code in the patches if that's okay with you. Sure - take my comments as that the code needs comments explaining what it tries to do. Apparently I misunderstood the intent (and still don't get it, but I don't remember the part in detail either). > >>> Overall this part of the rust frontend looks OK. Take the comments as > >>> suggestions (for future > >>> enhancements). > > Which seems to be the case :) > > The v4 of patches, which contains a lot of fixes for the issues you > mentioned, will be sent soon. > > All the best, > > Arthur