I agree the code is ugly... ;) But it shouldn't cause an ICE anyway
Created attachment 11493 [details] Preprocessor output
What target is this on?
The source you added is not free or open source.
Reduced testcase: struct st{ int _mark; }; unsigned long long t = ((int)&(((struct st*)16)->_mark) - 16); This is undefined code.
looks like creative offsetof
It is creative offsetof indeed -- this a "explanation" from a bit of code referenced to this one [while it isn't free yet, its license does allow posting bits of it online]: // HACK: gcc warns about applying offsetof() to non-POD object or calculating // offset directly when base address is NULL. Use 16 to get around the // warning. gcc-3.4 has an option -Wno-invalid-offsetof to suppress // this warning. #define offset_of(klass,field) (size_t)((intx)&(((klas*)16)->field) - 16) Looks like someone was desperate to get his stuff to compile with -Werror without having to fix the cause.
Subject: Re: [4.1/4.2 Regression] internal compiler error: no-op convert from 4 to 8 bytes in initializer > > > > ------- Comment #6 from bero at arklinux dot org 2006-05-23 21:41 ------- > It is creative offsetof indeed -- this a "explanation" from a bit of code > referenced to this one [while it isn't free yet, its license does allow posting > bits of it online]: > Looks like someone was desperate to get his stuff to compile with -Werror > without having to fix the cause. Well it is undefined for non-PODs to do offsetof which is why GCC warns. -- Pinski
Will not be fixed in 4.1.1; adjust target milestone to 4.1.2.
Eric -- Here, valid_initializer_constant_p returns true -- but output_constant aborts, saying: /* Make sure eliminating the conversion is really a no-op, except with VIEW_CONVERT_EXPRs to allow for wild Ada unchecked conversions and union types to allow for Ada unchecked unions. */ if (type_size > op_size && TREE_CODE (exp) != VIEW_CONVERT_EXPR && TREE_CODE (TREE_TYPE (exp)) != UNION_TYPE) internal_error ("no-op convert from %wd to %wd bytes in initializer", op_size, type_size); I don't understand the assertion, given that removing it seems to generate correct output for this test case. Since you edited this code not to long ago, do you have thoughts? -- Mark
The error orginally came from: 2005-09-01 DJ Delorie <dj@redhat.com> * varasm.c (output_constant): Let the target resolve conversions of addresses to non-default pointer sizes. And then there were a couple of changes after that.
> I don't understand the assertion, given that removing it seems to generate > correct output for this test case. Since you edited this code not to long ago, > do you have thoughts? Not really. I've only loosened it since it broke Ada. This is in DJ's court.
Subject: Re: [4.1/4.2 Regression] internal compiler error: no-op convert from 4 to 8 bytes in initializer > I don't understand the assertion, given that removing it seems to > generate correct output for this test case. The problem is that this function PADS rather than EXTENDS data that's shorter than it's supposed to be, and it always pads in the same direction - IIRC little endian. Thus, it breaks on big endian machines, or machines that require sign extension.
Subject: Re: [4.1/4.2 Regression] internal compiler error: no-op convert from 4 to 8 bytes in initializer dj at redhat dot com wrote: > ------- Comment #12 from dj at redhat dot com 2006-06-15 15:19 ------- > Subject: Re: [4.1/4.2 Regression] internal compiler error: no-op convert from > 4 to 8 bytes in initializer > > >> I don't understand the assertion, given that removing it seems to >> generate correct output for this test case. > > The problem is that this function PADS rather than EXTENDS data that's > shorter than it's supposed to be, and it always pads in the same > direction - IIRC little endian. Thus, it breaks on big endian > machines, or machines that require sign extension. So, you believe that initializer_constant_valid_p should reject this initializer then?
First off, I was mostly just explaining that assertion. Second, I think that this code can be a valid initializer - the value we initialize to is, in this case, an 8 byte value which contains the least significant 4 bytes of an offset. Because of the masking, we can't expect the assembler to figure it out, but the value happens to fit in a 4 byte value (er, unless it's a HUGE structure ;) so the compiler should be able to. However, that doesn't mean it's a no-op conversion. What I mean here, is that we can't rely on the code where the assert is to do the "conversion" for us, because it does it wrong. That code is designed to pad structures and such, not do type conversions. If you give it a 4 byte initializer, and ask for an 8 byte space to be filled, it pads. It does NOT actually convert anything, which causes the problems I noted. The reason I discovered this is because it was padding out an ADDRESS on the m16c (pointers are smaller than the address space), and the address was truncated. Thus, not truly a no-op conversion. So, I added code to try to detect the cases where we *can* let the assembler do the right thing (i.e. if there's a .op for the larger type), and reject the cases that we know we can't handle. The solution is to either add more special cases to the code where the assertion is, or actually do the conversions beforehand (something like simplify_rtx, but for trees, and perhaps knowing about signed vs unsigned). I think the optimizer could do this for this case, because it knows that the resulting value is constrained to be known to fit and not be truncated by the conversions, but I don't think that output_constant should be expected to do those kinds of optimizations.
DJ -- Thanks for the detailed comments. I understand that the assertion is guarding against some cases where we may silently generate wrong code (as with the situation with pointers on the m16c). However, we're now in the situation where he have a mismatch between initializer_constant_valid_p and output_constant, which is a recipe for ICEs. It is a key invariant that anything accepted by the former be something that can be output by the latter. So, we need to figure out what to do. My tentative inclination, given where we are in the development cycle, is to remove your assertion. We will generate correct code much of the time (as, for example, in this test case, where zero-extension is fine). Another choice would be to reject this case as an initializer, which will result in QoI regressions where initializers are now performed dynamically instead of statically. I agree that your strategy of simplifying valid initializers to make them obviously valid is best -- but I'm not sure we can easily implement that. What do you think? -- Mark
I think it's acceptable to temporarily disable that assertion for this release, but (1) we should test that code on both big and little endian 64 bit machines and see if it produces wrong code (perhaps with a larger offset to see if the truncation is happening), and (2) we should consider re-enabling the assertion later if so. Ideally, the assertion only detects the case where it *will* produce wrong code, at least on a big endian machine. That might imply that more special cases are needed for what is truly a no-op conversion, or an extra optimization step before we check the initializers.
I don't think disabling the assertion is enough. Yes, the compiler would do the right thing for this particular testcase, but would be wrong if the subtraction produces a negative result. Rather, we should stop stripping the conversions in cases where they are significant. Testing a patch now.
Subject: Bug 27724 Author: jason Date: Fri Sep 8 05:11:40 2006 New Revision: 116777 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=116777 Log: PR middle-end/27724 * varasm.c (output_constant): Only strip actual no-op conversions. Added: trunk/gcc/testsuite/gcc.dg/long-long-cst1.c Modified: trunk/gcc/ChangeLog trunk/gcc/varasm.c
Subject: Bug 27724 Author: jason Date: Fri Sep 8 05:16:26 2006 New Revision: 116778 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=116778 Log: PR middle-end/27724 * varasm.c (output_constant): Only strip actual no-op conversions. Added: branches/gcc-4_1-branch/gcc/testsuite/gcc.dg/long-long-cst1.c - copied unchanged from r116777, trunk/gcc/testsuite/gcc.dg/long-long-cst1.c Modified: branches/gcc-4_1-branch/gcc/ChangeLog branches/gcc-4_1-branch/gcc/varasm.c
fixed in 4.1 as well.