Quoting nathan binkert <[email protected]>:
goto! Yikes! This needs to go! It makes the flow of this function
hard to follow, and I'm sure it's not doing the compiler any favors.
For the record, I think this knee-jerk reaction to goto is completely
misguided. Goto is frequently wrong, but when it is used as an escape
function (e.g. like break), I believe that it is totally justified.
Any time you try to avoid a goto in these situations, you increase the
level of nesting by writing an if-statment. My belief is that goto is
ideal for escaping more than one level of looping (some languages have
explicit support for this), or to execute error handling code before
returning from a function.
I wouldn't classify this as knee jerk at all. Ali has put in a (small)
number of gotos for error handling sort of situations and while it's a
little unpleasant I can see the reasoning. Here we're using it to jump
between a construct in an if into the body of the else, and that's
just not how those constructs are supposed to work. When this function
ends up 5 times this long, I'm not going to read all of it. I'm going
to see an if and expect its condition dictates whether the code inside
it is executed. We would be breaking that very reasonable assumption
and making it much harder to see what the code is doing without
studying it carefully.
Actually I don't think it's hard to follow at all. Normally we pick the
next available address if the user doesn't provide one, and in the
somewhat exceptional case that the user provided an address *but*
part of the range is already in use *and* the MAP_FIXED flag wasn't
specified to force an override of that prior use, then you also fall
back on that policy. Obviously the goto isn't strictly necessary,
but IMO rewriting to avoid the goto does not make the logic more
apparent or easier to follow (if anything, the opposite).
I actually don't think it's that hard to follow, but I agree that it's
strange. I think a common way to write this would be to use a flag
variable that's false to start, gets set to true in the one place, and
then replace the } else { chunk of code with an if (flag) {
You are right that goto can have a negative impact on compiler optimization,
but if you're spending a measurable fraction of your runtime calling mmap()
then you have bigger problems than suboptimal simulation speed.
That depends. If you use goto to replace normal looping stuff, yes,
but if in the end, you need to replace the goto with a bunch of extra
if-statements, it may not matter.
There aren't any loops here, but we're still cutting between the
branches of the control flow graph in a strange way. I don't remember
what they are, but there are things the compiler can't do any more
when you do that sort of thing.
Nate
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
