From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id D91043858C35 for ; Thu, 9 May 2024 05:06:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D91043858C35 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org D91043858C35 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1715231201; cv=none; b=C1n1Fn5Nig++IIdU7YCDX38cQp3AGyRUDqJBVHlDzbxzEGW/3Jcs5uhf9u6UDXOYFmqmwWLhzTER0FzdeV62xLRAYZVxCOyNULjApbQWqZSL0ccjlmeSEvvm39mJWsD3EESig8nmEmcx1c6fuV+Fse24Hfp/yG4sKteY/qBOFvU= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1715231201; c=relaxed/simple; bh=WC7cAK28YIX9CCWtUb65VgYJZzbY7wcAlqF6KGG0lUo=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=Q57H7y4vOoK/0ac9DYJaMr40r7nxFyKq+R9N5aNdUWdq6lzW1+ogFBNXvOB7iRKZN9EJvj123IAiI0OGbZ40qYrN4j+E/wsc40mbszYl7DE/ZKxAail8Hv9+34PmhGhEePIzzdhhx5m6SE84zQs3otVHqZLOBIY1dY24YRTsv0g= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1715231198; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ruxo+pi86dvZdrk5fyRjFNsdJuxEMGP8bG0Jy8CtS8U=; b=GXX/ZufEGido5rdfQ/ETay51LEDwaZ2D0mSZj3Ki7i2/8v5K+XzQrnqJTpY65FFRxiUXFK qMbIZl0DuOSMZEp6uu8keBgJ6oKSr/7Sl5z7aLdOcHoPJ9d+WZUMV3juHps31HQFQAIMvF Xmyvf2FFgAPldcimLq5W3kbvNK5nWqc= Received: from mail-ej1-f70.google.com (mail-ej1-f70.google.com [209.85.218.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-637-5MTETs0mPoKRhUL3X46JIw-1; Thu, 09 May 2024 01:06:36 -0400 X-MC-Unique: 5MTETs0mPoKRhUL3X46JIw-1 Received: by mail-ej1-f70.google.com with SMTP id a640c23a62f3a-a524b774e39so44699966b.1 for ; Wed, 08 May 2024 22:06:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1715231195; x=1715835995; h=content-transfer-encoding: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=ruxo+pi86dvZdrk5fyRjFNsdJuxEMGP8bG0Jy8CtS8U=; b=JNRP0MsiO7Bg6KzL4cGUZt+3SZMMgNp5+mDaYqeTPFkufNteGtlEnEOPZIDlYeYd/A +SIsoXBGR9TMcJJ9J/4NtmfuvBJZX4F69MXO9WRnSuANT2kXzymd/veA3HF6b38498ge LzE3YezQH7PCnNYmL93gPdWa5XZ+qRWSzTU2+DAIBoWAKNdMTm+VAmhkMg1tWcqclfm7 HOXoedAqZgJbRao3vXD2+QmOuyqLs0GlHs6NaGF3lrFQjcxUFWgWqSdMmu6vNl5+A3Js X5K7IVBj27pRPhN8VCcOLcnhJ0LthucPvgplh5X+B8XUb6kQ2nZDsTRaug4thtszf8Rp 3x8g== X-Gm-Message-State: AOJu0YwIZxOazbbGLq8e9skH6Q5KaXd/da98ic2FYSy3FRQPeBiIlNO5 5ePsfzIzuYWqmoMbOH7hRDUZ3QKKToeHodvRCEN8Y+QqWFlrgOTtcW+pHiCrqsnqDMh4I2/IeM+ skFMSyqU4iACTqDKmove5FErci0n6FwqGrWo3wkY+BZAtvdP/vF1l3eFFp5sUi0ECbtBHIXH8VO uw5EdiLmmLyreidzrJgHJJo5+NHhtjSg== X-Received: by 2002:a05:6402:270d:b0:573:307d:eb1 with SMTP id 4fb4d7f45d1cf-57332786ef2mr1434480a12.10.1715231194870; Wed, 08 May 2024 22:06:34 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGr9TZbgWKV8RrXnWV5mYu3SlqwIXpyfbavQmV/qAIld3gx6cVtzsGHe4Lm3AOLyViz63oF4cK2uu4W38NOwF8= X-Received: by 2002:a05:6402:270d:b0:573:307d:eb1 with SMTP id 4fb4d7f45d1cf-57332786ef2mr1434452a12.10.1715231194260; Wed, 08 May 2024 22:06:34 -0700 (PDT) MIME-Version: 1.0 References: <20240503092307.474116-1-aldyh@redhat.com> In-Reply-To: From: Aldy Hernandez Date: Thu, 9 May 2024 07:06:23 +0200 Message-ID: Subject: Re: [PATCH] [ranger] Force buffer alignment in Value_Range [PR114912] To: Andrew Pinski Cc: GCC patches , Andrew MacLeod X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,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: Pushed to trunk to unblock sparc. On Fri, May 3, 2024 at 4:24=E2=80=AFPM Aldy Hernandez wr= ote: > > Ahh, that is indeed cleaner, and there's no longer a need to assert > the sizeof of individual ranges. > > It looks like a default constructor is needed for the buffer now, but > only for the default constructor of Value_Range. > > I have verified that the individual range constructors are not called > on initialization to Value_Range, which was the original point of the > patch. I have also run our performance suite, and there are no > changes to VRP or overall. > > I would appreciate a review from someone more C++ savvy than me :). > > OK for trunk? > > On Fri, May 3, 2024 at 11:32=E2=80=AFAM Andrew Pinski = wrote: > > > > On Fri, May 3, 2024 at 2:24=E2=80=AFAM Aldy Hernandez wrote: > > > > > > Sparc requires strict alignment and is choking on the byte vector in > > > Value_Range. Is this the right approach, or is there a more canonica= l > > > way of forcing alignment? > > > > I think the suggestion was to change over to use an union and use the > > types directly in the union (anonymous unions and unions containing > > non-PODs are part of C++11). > > That is: > > union { > > int_range_max int_range; > > frange fload_range; > > unsupported_range un_range; > > }; > > ... > > m_vrange =3D new (&int_range) int_range_max (); > > ... > > > > Also the canonical way of forcing alignment in C++ is to use aliagnas > > as my patch in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D114912 > > did. > > Also I suspect the alignment is not word alignment but rather the > > alignment of HOST_WIDE_INT which is not always the same as the > > alignment of the pointer but bigger and that is why it is failing on > > sparc (32bit rather than 64bit). > > > > Thanks, > > Andrew Pinski > > > > > > > > If this is correct, OK for trunk? > > > > > > gcc/ChangeLog: > > > > > > * value-range.h (class Value_Range): Use a union. > > > --- > > > gcc/value-range.h | 24 +++++++++++++++--------- > > > 1 file changed, 15 insertions(+), 9 deletions(-) > > > > > > diff --git a/gcc/value-range.h b/gcc/value-range.h > > > index 934eec9e386..31af7888018 100644 > > > --- a/gcc/value-range.h > > > +++ b/gcc/value-range.h > > > @@ -740,9 +740,14 @@ private: > > > void init (const vrange &); > > > > > > vrange *m_vrange; > > > - // The buffer must be at least the size of the largest range. > > > - static_assert (sizeof (int_range_max) > sizeof (frange), ""); > > > - char m_buffer[sizeof (int_range_max)]; > > > + union { > > > + // The buffer must be at least the size of the largest range, an= d > > > + // be aligned on a word boundary for strict alignment targets > > > + // such as sparc. > > > + static_assert (sizeof (int_range_max) > sizeof (frange), ""); > > > + char m_buffer[sizeof (int_range_max)]; > > > + void *align; > > > + } u; > > > }; > > > > > > // The default constructor is uninitialized and must be initialized > > > @@ -816,11 +821,11 @@ Value_Range::init (tree type) > > > gcc_checking_assert (TYPE_P (type)); > > > > > > if (irange::supports_p (type)) > > > - m_vrange =3D new (&m_buffer) int_range_max (); > > > + m_vrange =3D new (&u.m_buffer) int_range_max (); > > > else if (frange::supports_p (type)) > > > - m_vrange =3D new (&m_buffer) frange (); > > > + m_vrange =3D new (&u.m_buffer) frange (); > > > else > > > - m_vrange =3D new (&m_buffer) unsupported_range (); > > > + m_vrange =3D new (&u.m_buffer) unsupported_range (); > > > } > > > > > > // Initialize object with a copy of R. > > > @@ -829,11 +834,12 @@ inline void > > > Value_Range::init (const vrange &r) > > > { > > > if (is_a (r)) > > > - m_vrange =3D new (&m_buffer) int_range_max (as_a (r)); > > > + m_vrange =3D new (&u.m_buffer) int_range_max (as_a (r))= ; > > > else if (is_a (r)) > > > - m_vrange =3D new (&m_buffer) frange (as_a (r)); > > > + m_vrange =3D new (&u.m_buffer) frange (as_a (r)); > > > else > > > - m_vrange =3D new (&m_buffer) unsupported_range (as_a (r)); > > > + m_vrange > > > + =3D new (&u.m_buffer) unsupported_range (as_a (r)); > > > } > > > > > > // Assignment operator. Copying incompatible types is allowed. Tha= t > > > -- > > > 2.44.0 > > > > >