| Message ID | 4F100499.7060702@gjlay.de |
|---|---|
| State | New |
| Headers | show |
On Jan 13, 2012, at 2:16 AM, Georg-Johann Lay wrote: > Fixed as obvious as 1 << 15 fits always inside an int whereas 1<<16 does not. Fixed as obvious is usually the term we use when work is checked in. > Ok for trunk? Ok for trunk is the term we use for work that hasn't been checked in. So, I'm left wondering if the work is in or not. Anyway, after reviewing the PR, it seems like they need a large enough offset to require 4 bytes of displacement size, and 1<<15 I suspect isn't actually large enough. I think the proposed change removes a valuable test, so I don't think it is ok. Instead, skip the testcase on int16 systems, or require int32 and larger systems... Ok with that change. > * gcc.dg/debug/dwarf2/pr49871.c: Fix to work on int16 platforms. > > > Index: gcc.dg/debug/dwarf2/pr49871.c > =================================================================== > --- gcc.dg/debug/dwarf2/pr49871.c (revision 183150) > +++ gcc.dg/debug/dwarf2/pr49871.c (working copy) > @@ -4,7 +4,7 @@ > > struct S > { > - char a[1 << 16]; > + char a[1 << 15]; > int b; > } s;
On Fri, Jan 13, 2012 at 09:12:25AM -0800, Mike Stump wrote: > On Jan 13, 2012, at 2:16 AM, Georg-Johann Lay wrote: > > Fixed as obvious as 1 << 15 fits always inside an int whereas 1<<16 does not. > > Fixed as obvious is usually the term we use when work is checked in. > > > Ok for trunk? > > Ok for trunk is the term we use for work that hasn't been checked in. So, I'm left wondering if the work is in or not. > > Anyway, after reviewing the PR, it seems like they need a large enough > offset to require 4 bytes of displacement size, and 1<<15 I suspect isn't > actually large enough. I think the proposed change removes a valuable > test, so I don't think it is ok. Yeah, this change is definitely wrong. Georg-Johann, please, any time you propose similar changes always try to reproduce the bug that was actually being fixed by the PR in the question, with some older and newer compiler, and see if your changes don't make the testcase useless as in this case. Obviously changes to require int32plus or similar don't need such investigation. Jakub
Index: gcc.dg/debug/dwarf2/pr49871.c =================================================================== --- gcc.dg/debug/dwarf2/pr49871.c (revision 183150) +++ gcc.dg/debug/dwarf2/pr49871.c (working copy) @@ -4,7 +4,7 @@ struct S { - char a[1 << 16]; + char a[1 << 15]; int b; } s;