Message ID | 1486156.RQZsauqaQ0@polaris |
---|---|
State | New |
Headers | show |
On Sat, Dec 8, 2012 at 12:42 PM, Eric Botcazou <ebotcazou@adacore.com> wrote: > This is an internal error in for_each_index at -O: > > +===========================GNAT BUG DETECTED==============================+ > | 4.8.0 20121208 (experimental) [trunk revision 194319] (x86_64-suse-linux) > GCC error:| > | in for_each_index, at tree-ssa-loop-im.c:324 | > | Error detected around vect9.adb:45:1| > > The callback is invoked on a vector CONST_DECL and stops. That's an unusual > situation (the CONST_DECL is wrapped up in a VIEW_CONVERT_EXPR to array type) > but IMO there is no reason to accept other DECLs and stop on CONST_DECLs. > > Tested on x86_64-suse-linux, applied on the mainline as obvious. Well ... I would have expected that we'd have folded the CONST_DECL to its DECL_INITIAL. At least that is what we do for all other CONST_DECLs. The only way CONST_DECLs should appear in the IL (after some optimization of course) is when you take their address. So - if you can burn some extra cycles to figure out why it's not replaced by its DECL_INITIAL ...? Richard. > 2012-12-08 Eric Botcazou <ebotcazou@adacore.com> > > * tree-ssa-loop-im.c (for_each_index) <CONST_DECL>: New case. > > > 2012-12-08 Eric Botcazou <ebotcazou@adacore.com> > > * gnat.dg/vect9.ad[sb]: New test. > * gnat.dg/vect9_pkg.ads: New helper. > > > -- > Eric Botcazou
> Well ... I would have expected that we'd have folded the CONST_DECL to > its DECL_INITIAL. At least that is what we do for all other CONST_DECLs. > The only way CONST_DECLs should appear in the IL (after some optimization > of course) is when you take their address. That's what (essentially) happens here. The CONST_DECL is wrapped up in a VIEW_CONVERT_EXPR to array type and a component with non-constant index is extracted from it. > So - if you can burn some extra cycles to figure out why it's not > replaced by its DECL_INITIAL ...? We specifically prevent that in gigi because it will need to be spilled to memory given the above access pattern, so I don't really see the problem. Here's the access in .optimized: _38 = MEM[symbol: pkg__zero_unit, index: ivtmp.38_27, step: 4, offset: -4B];
On Mon, Dec 10, 2012 at 10:55 AM, Eric Botcazou <ebotcazou@adacore.com> wrote: >> Well ... I would have expected that we'd have folded the CONST_DECL to >> its DECL_INITIAL. At least that is what we do for all other CONST_DECLs. >> The only way CONST_DECLs should appear in the IL (after some optimization >> of course) is when you take their address. > > That's what (essentially) happens here. The CONST_DECL is wrapped up in a > VIEW_CONVERT_EXPR to array type and a component with non-constant index is > extracted from it. Ah, ok ... >> So - if you can burn some extra cycles to figure out why it's not >> replaced by its DECL_INITIAL ...? > > We specifically prevent that in gigi because it will need to be spilled to > memory given the above access pattern, so I don't really see the problem. > Here's the access in .optimized: > > _38 = MEM[symbol: pkg__zero_unit, index: ivtmp.38_27, step: 4, offset: -4B]; ... so if would really be a pessimization doing that. Of course handling CONST_DECL in for_each_index is indeed obvious - I just was curios if it was a missed-optimization opportunity as well. Thanks for clarifying, Richard. > -- > Eric Botcazou
Index: tree-ssa-loop-im.c =================================================================== --- tree-ssa-loop-im.c (revision 194319) +++ tree-ssa-loop-im.c (working copy) @@ -291,6 +291,7 @@ for_each_index (tree *addr_p, bool (*cbc case VAR_DECL: case PARM_DECL: + case CONST_DECL: case STRING_CST: case RESULT_DECL: case VECTOR_CST: