From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id AE7B8398B856; Wed, 16 Jun 2021 09:46:32 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AE7B8398B856 From: "rguenther at suse dot de" To: gcc-bugs@gcc.gnu.org Subject: [Bug tree-optimization/101061] tree-vrp misoptimization on skylake+ using union-based aliasing Date: Wed, 16 Jun 2021 09:46:32 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: tree-optimization X-Bugzilla-Version: 8.5.0 X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: rguenther at suse dot de X-Bugzilla-Status: UNCONFIRMED X-Bugzilla-Resolution: X-Bugzilla-Priority: P3 X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 X-BeenThere: gcc-bugs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-bugs mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Jun 2021 09:46:32 -0000 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D101061 --- Comment #11 from rguenther at suse dot de --- On Wed, 16 Jun 2021, alexander.grund@tu-dresden.de wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D101061 >=20 > --- Comment #9 from Alexander Grund --- > > Note that when the union type is visible in the access path then GCC al= lows punning without further restrictions. Thus the accesses as written abo= ve are OK. >=20 > Now I have to ask again for clarification because this seemingly contradi= cts > your previous statement. So let me try to state the previous question aga= in: >=20 > Given a pointer `slot_type* slot` (where slot_type is the union as defined > before) is it legal to access `slot->value.first`, `slot->mutable_value.f= irst`, > `slot->key` interchangeably? >=20 > In all 3 accesses the union is visible in the access path (if I understoo= d that > correctly) so the type punning should be ok, shouldn't it? Yes - but it routinely happens that in C++ you end up returning a reference to the union member as abstraction and an access to that=20 reference does _not_ have the union visible but only the member. > > the circumstances have been so there's incentive to do an invalid trans= form...=20 >=20 > So is this indeed a GCC bug which may be fixed or gone latent? I don't think so, but we cannot rule this out at this point (but see=20 above). > Otherwise, maybe the construction is the problem? What Abseil basically=20 > does is allocating a (suitably aligned) buffer of chars and in-place=20 > constructing those slot_type unions/pairs there as needed (i.e. similar=20 > to std::vector) After abstractions what basically happens is: >=20 > // alloc buffer done, then: > slot_type* slot_buffer =3D reinterpret_cast(buffer); >=20 > // on emplace: > size_t x =3D ...; > slot_type* slot =3D slot_buffer + x; > new(slot) slot_type; > new(&slot->mutable_value) pair(k, v); > slot_type* slot2 =3D slot_buffer + x; // same as before, actually done th= rough > the iterator > idx_vec[i] =3D slot2->value.second; so if slot_type is the union type then new(&slot->mutable_value) pair(k, v) looks problematic since that calls operator new with a pointer to the union member and the actual construction receives &slot->mutable_value as address of type decltype (slot->mutable_value) * which falls foul of the union punning rule. I'm not sure how one can solve this issue with using placement new but are unions not sufficiently restricted so that copy assignment should work (and activate the appropriate union member)? Thus slot->mutable_value =3D pair(k, v); ? The compiler should hopefully be able to elide the copy. > My suspicion is, that GCC loads the value of slot2 before constructing the > object, at least sometimes. Printing the values in the vector often shows= a run > of zeroes before some other (potentially correct) values. I'd guess the c= orrect > values happen, when no insertion took place, i.e. the construction was do= ne > already in a prior loop iteration. Yes, GCC would simply "skip" the offending placement new and if it finds a suitable definition before it would replace the load with said=20 definition. To expand on the placement new issue if you for example have struct Y { Y(int); }; union X { int i; float f; }; void foo (void *p) { X *q =3D reinterpret_cast (p); new (&q->i) Y (1); } we end up with (early unoptimized IL): void __GIMPLE (void * p) { void * D_6558; void * D_6557; union X * q; q =3D p; D_6558 =3D &q->i; D_6557 =3D operator new (1ul, D_6558); try { Y::Y (D_6571, 1); } catch { operator delete (D_6557, D_6558); } } where 'operator new' is the usual noop that just returns the passed pointer here. But what you can see is that the resulting pointer is used for the initialization and not the placement address as literally written in source. And even then, the CTOR that is involved of course sees only 'this' which is a plain pointer to its class type and all accesses in the CTOR will not have the union visible. That might be unexpected but I think it's a quite natural consequence of C++ abstraction :/=