This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch, fortran] PR22539 implement FSEEK intrinsic


Brooks Moses wrote:
At 02:37 PM 5/3/2007, Daniel Franke wrote:
On Thursday 03 May 2007 23:22:05 Brooks Moses wrote:
> I do have one other question, which I apologize for neglecting to
> mention earlier: you mentioned, at some point, handling various kinds of
> the STATUS variable by some form of conversion, if I'm remembering
> correctly. I don't see any of that in the patch, though, and it looks
> like you're constructing a call that passes along the STATUS argument
> with whatever kind it happens to be, and then fseek_sub expects an int*.


I mentioned, that the conversion still needs to be done, not that it is
available yet :)

Ah, okay. :)


That is the very same problem as other intrinsics have, e.g. EXIT, SIGNAL, you
name it. Most, if not all, that feature a status variable. I still plan to
find a general fix for the problem and to get rid of the related foo_i[1248]
functions in the library by arranging for appropiate casting before and after
the call, respectively. I'm still not sure on how to accomplish this as I am
sort of lost with the creation of gimple, but eventually I'll figure it out.
Thus, I concluded that there is no need to water-tight FSEEK in this respect
as there are other functions/subroutines -- that are used more often -- that
still need to be fixed. I.e. I plan to get back to it as soon as I know how
to create the appropriate casts. Is this acceptable for now?

I'd much prefer to see it "water-tight" for now, especially since it's only a quick three-line fix. At the very least, the documentation should record the actual state of things.


Part of this is that, as you say, finding a general fix is a fairly hard problem. Currently you're planning on working on that, but plans change and other things become more important, and it may turn out to be a lot harder than you expected, and so there are no guarantees. Thus, I think it's important to fix things so that they'll be okay even if the future plans don't happen the way we expect.

(Also, this is a regression, and we may backport it to 4.2 on those grounds -- I would be tempted to do that just to make it easier to keep the documentation synchronized. If we do that, we won't be backporting the "global fix".)

One other advantage of putting in the "quick fix" is that it records that this is something that needs to be changed later when we have a global fix for the problem. If I were putting in a global fix for this, one of the things I'd be doing is looking through check.c for all the cases where the kind of a variable is compared to 4, or to gfc_default_integer_kind....

But, yeah, I definitely agree that you shouldn't put a lot of working into making it "water-tight" -- if I actually expected this to stay the way it is for the long term, I'd be telling you to make it work properly to handle both kind=4 and kind=8 default integers and all that. Limiting it to kind=4 is just the quick fix that keeps the "leak" plugged until someone (hopefully you!) gets a chance to do the appropriate global fix for everything. :)

- Brooks


I don't think the extra check for water-tight is that critical, but it's easy to do. I think we all know how difficult the gimples and the trees are. :)

This is my approval to commit with Brooks check tweak.

As a possible help for your next phase, look in trans_io.c. There are examples of checking and converting the UNIT number in set_parameter_value. If I understand correctly, you set the destination type and then call convert.

Thanks,

Jerry


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]