| Message ID | 20100920222130.27269.19670@gimli.local |
|---|---|
| State | New |
| Headers | show |
Mikael, This is OK except that I do not understand what the gcc_assert is doing in: + + /* look for the corresponding scalarizer dimension: dim. */ + for (dim = 0; dim < ndim; dim++) + if (info->dim[dim] == n) + break; + gcc_assert (dim < ndim); Have I missed something? Cheers Paul On 9/21/10, Mikael Morin <mikael.morin@sfr.fr> wrote: > We can't have dim (new descriptor's dimension) increasing together with n > (original array's dimension) if we want to support transposed reference. We > have to get dim such that info->dim[dim] == n. Then we can set the > descriptor bounds along dim. > > OK for trunk? > > >
Paul Richard Thomas wrote: > Mikael, > > This is OK except that I do not understand what the gcc_assert is doing in: > + > + /* look for the corresponding scalarizer dimension: dim. */ > + for (dim = 0; dim < ndim; dim++) > + if (info->dim[dim] == n) > + break; > + gcc_assert (dim < ndim); > > Have I missed something? This seems like checking the loop is exited early in any case. (But I've no idea about the rest of the patch.) Daniel
Dominique, The loop takes dim from 0 to ndim-1. Therefore the condition of the gcc_assert is always going to be satisfied. Cheers Paul On 9/21/10, Daniel Kraft <d@domob.eu> wrote: > Paul Richard Thomas wrote: >> Mikael, >> >> This is OK except that I do not understand what the gcc_assert is doing >> in: >> + >> + /* look for the corresponding scalarizer dimension: dim. */ >> + for (dim = 0; dim < ndim; dim++) >> + if (info->dim[dim] == n) >> + break; >> + gcc_assert (dim < ndim); >> >> Have I missed something? > > This seems like checking the loop is exited early in any case. (But > I've no idea about the rest of the patch.) > > Daniel > > -- > http://www.pro-vegan.info/ > -- > Done: Arc-Bar-Cav-Kni-Ran-Rog-Sam-Tou-Val-Wiz > To go: Hea-Mon-Pri >
s/Dominique/Daniel/ :-) Paul On 9/21/10, Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote: > Dominique, > > The loop takes dim from 0 to ndim-1. Therefore the condition of the > gcc_assert is always going to be satisfied. > > Cheers > > Paul > > On 9/21/10, Daniel Kraft <d@domob.eu> wrote: >> Paul Richard Thomas wrote: >>> Mikael, >>> >>> This is OK except that I do not understand what the gcc_assert is doing >>> in: >>> + >>> + /* look for the corresponding scalarizer dimension: dim. */ >>> + for (dim = 0; dim < ndim; dim++) >>> + if (info->dim[dim] == n) >>> + break; >>> + gcc_assert (dim < ndim); >>> >>> Have I missed something? >> >> This seems like checking the loop is exited early in any case. (But >> I've no idea about the rest of the patch.) >> >> Daniel >> >> -- >> http://www.pro-vegan.info/ >> -- >> Done: Arc-Bar-Cav-Kni-Ran-Rog-Sam-Tou-Val-Wiz >> To go: Hea-Mon-Pri >> > > > -- > The knack of flying is learning how to throw yourself at the ground and > miss. > --Hitchhikers Guide to the Galaxy >
On Tue, Sep 21, 2010 at 4:46 PM, Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote: > Dominique, > > The loop takes dim from 0 to ndim-1. Therefore the condition of the > gcc_assert is always going to be satisfied. But if the loop terminates via its exit condition !(dim<ndim) then obviously dim will be not < ndim ;) RIchard. > Cheers > > Paul > > On 9/21/10, Daniel Kraft <d@domob.eu> wrote: >> Paul Richard Thomas wrote: >>> Mikael, >>> >>> This is OK except that I do not understand what the gcc_assert is doing >>> in: >>> + >>> + /* look for the corresponding scalarizer dimension: dim. */ >>> + for (dim = 0; dim < ndim; dim++) >>> + if (info->dim[dim] == n) >>> + break; >>> + gcc_assert (dim < ndim); >>> >>> Have I missed something? >> >> This seems like checking the loop is exited early in any case. (But >> I've no idea about the rest of the patch.) >> >> Daniel >> >> -- >> http://www.pro-vegan.info/ >> -- >> Done: Arc-Bar-Cav-Kni-Ran-Rog-Sam-Tou-Val-Wiz >> To go: Hea-Mon-Pri >> > > > -- > The knack of flying is learning how to throw yourself at the ground and miss. > --Hitchhikers Guide to the Galaxy >
On 09/21/2010 04:46 PM, Paul Richard Thomas wrote: > The loop takes dim from 0 to ndim-1. Therefore the condition of the > gcc_assert is always going to be satisfied. I disagree. The assert triggers a) If ndim < 1 (and the loop is never entered) b) If one does not have an early exit - as Daniel pointed out: #include <stdio.h> main () { int i; for (i = 0; i < 5; i++) ; printf("%d\n", i); return 0; } Running the program prints "5". Tobias > On 9/21/10, Daniel Kraft<d@domob.eu> wrote: >> Paul Richard Thomas wrote: >>> + for (dim = 0; dim< ndim; dim++) >>> + if (info->dim[dim] == n) >>> + break; >>> + gcc_assert (dim< ndim); >> This seems like checking the loop is exited early in any case. (But >> I've no idea about the rest of the patch.)
Dear All,
> b) If one does not have an early exit - as Daniel pointed out:
duuuuh!
Paul
On Tuesday 21 September 2010 17:03:54 Paul Richard Thomas wrote: > Dear All, > > > b) If one does not have an early exit - as Daniel pointed out: > duuuuh! > > Paul Hello all, yes, the assert checks the loop was exited early, and thus the dim looked for has been found. Would there be a way to present this more clearly ? How ? Mikael
On Tuesday 21 September 2010 17:41:15 Mikael Morin wrote: > On Tuesday 21 September 2010 17:03:54 Paul Richard Thomas wrote: > > Dear All, > > > > > b) If one does not have an early exit - as Daniel pointed out: > > duuuuh! > > > > Paul > > Hello all, > > yes, the assert checks the loop was exited early, and thus the dim looked > for has been found. > Would there be a way to present this more clearly ? How ? I will add a comment.
On Tue, Sep 21, 2010 at 05:41:15PM +0200, Mikael Morin wrote: > On Tuesday 21 September 2010 17:03:54 Paul Richard Thomas wrote: > > Dear All, > > > > > b) If one does not have an early exit - as Daniel pointed out: > > duuuuh! > > > > Paul > > Hello all, > > yes, the assert checks the loop was exited early, and thus the dim looked for > has been found. > Would there be a way to present this more clearly ? How ? > Well, C has a feature called a comment. /* Assert that the loop exited early. */ gcc_assert (...); :-)
Steve, Mikael, I am not sure that it is necessary to compensate for my suffering temporary brain failure :-) It is about as obvious as can be..... Cheers Paul On Tue, Sep 21, 2010 at 6:10 PM, Steve Kargl <sgk@troutmask.apl.washington.edu> wrote: > On Tue, Sep 21, 2010 at 05:41:15PM +0200, Mikael Morin wrote: >> On Tuesday 21 September 2010 17:03:54 Paul Richard Thomas wrote: >> > Dear All, >> > >> > > b) If one does not have an early exit - as Daniel pointed out: >> > duuuuh! >> > >> > Paul >> >> Hello all, >> >> yes, the assert checks the loop was exited early, and thus the dim looked for >> has been found. >> Would there be a way to present this more clearly ? How ? >> > > Well, C has a feature called a comment. > > /* Assert that the loop exited early. */ > gcc_assert (...); > > :-) > > -- > Steve >
diff --git a/trans-array.c b/trans-array.c index 7bce2ef..f15b53e 100644 --- a/trans-array.c +++ b/trans-array.c @@ -5439,7 +5439,6 @@ gfc_conv_expr_descriptor (gfc_se * se, gfc_expr * expr, gfc_ss * ss) } offset = gfc_index_zero_node; - dim = 0; /* The following can be somewhat confusing. We have two descriptors, a new one and the original array. @@ -5479,9 +5478,6 @@ gfc_conv_expr_descriptor (gfc_se * se, gfc_expr * expr, gfc_ss * ss) } else { - /* Check we haven't somehow got out of sync. */ - gcc_assert (info->dim[dim] == n); - /* Evaluate and remember the start of the section. */ start = info->start[n]; stride = gfc_evaluate_now (stride, &loop.pre); @@ -5505,6 +5501,12 @@ gfc_conv_expr_descriptor (gfc_se * se, gfc_expr * expr, gfc_ss * ss) /* Vector subscripts need copying and are handled elsewhere. */ if (info->ref) gcc_assert (info->ref->u.ar.dimen_type[n] == DIMEN_RANGE); + + /* look for the corresponding scalarizer dimension: dim. */ + for (dim = 0; dim < ndim; dim++) + if (info->dim[dim] == n) + break; + gcc_assert (dim < ndim); /* Set the new lower bound. */ from = loop.from[dim]; @@ -5559,8 +5561,6 @@ gfc_conv_expr_descriptor (gfc_se * se, gfc_expr * expr, gfc_ss * ss) /* Store the new stride. */ gfc_conv_descriptor_stride_set (&loop.pre, parm, gfc_rank_cst[dim], stride); - - dim++; } if (se->data_not_needed)