SSA_NAMES: should there be an unused, un-free limbo?
In my ongoing plumbing work on Dan Berlin's Operator Strength Reduction implementation, I've encountered what seems to me like a bug. On IRC yesterday everyone seemed to agree that it was a bug. Zdenek, OTOH, says it is not a bug. I would like it to be a bug (so it will get fixed everywhere), so I'm interested in getting consensus here. Hopefully Zdenek will speak up and explain why he thinks it needs to be allowed. The situation is that some SSA_NAMEs are disused (removed from the code) without being released onto the free list by release_ssa_name(). I have found only one place where this happens, but Zdenek claims there are other places. The result is that it is apparently impossible to tell by iterating over the vector of SSA_NAMEs whether any particular SSA_NAME is currently in use by the code. What I would like is for the SSA_NAME_IN_FREE_LIST bit to indicate reliably whether the SSA_NAME is unused. If there is no way to deduce what I want by looking at an SSA_NAME, it will be necessary to walk over the code whenever we simply want to iterate over SSA_NAMEs that are in use. Please discuss. -- Robert Kennedy
Re: SSA_NAMES: should there be an unused, un-free limbo?
> I think the discussion should begin with reevaluating whether or not > the memory savings from recycling SSA_NAMEs is still worth the headache. That's a separate project that I'd rather not bundle with strength reduction, because the two are unrelated conceptually. My opinion is that instead, the following question should be the starting point: Some code in GCC has a demonstrable need to iterate over the SSA_NAMEs in current use by the program being compiled. Do we want to provide a reasonable way to do that iteration? Currently we don't provide it. That seems wrong to me. I don't much care about the details of *how* the facility is provided, although if implementing it doesn't divert me too much I'm happy to do it. Plenty of people who know more than I do will have opinions about how it should be done, and that's fine. I don't want to decide how it should be done, necessarily. I would simply like to begin with agreement that it should be done. Or, if the consensus is that it should not be done in spite of the need, some reasonable advice about what to do instead (I don't see a good alternative). -- Robert
Re: SSA_NAMES: should there be an unused, un-free limbo?
I wrote: > I don't much care about the details of *how* the facility is provided... I should rephrase this. I do care about the interface to it. I don't care so much about the implementation. I would like the interface to be straightforward. I don't want to do a separate project to reevaluate the need for SSA_NAME recycling and build consensus about that as a prerequisite. -- Robert
Re: SSA_NAMES: should there be an unused, un-free limbo?
> In this case this isn't true, because we have code that orphans ssa > names without releasing them. > I'm sure Robert will explain further details in a few moments :) Actually you explained all the relevant details. The question is whether it should be allowed or not. So far almost everyone seems to think not. > It will cause not code correctness issues, but make life very very > annoying. If you walk the ssa names one by one, and you have some > pointing to statements that don't exist anymore, because the names > were not released and defining statements nulled out by > release_ssa_name, you are going to run into segfaults trying to walk > them. It isn't specifically important that the defining statements be nulled out, of course. What matters is that there should be *some way* to tell whether the item is in current use. Incidentally, checking for a NULL defining statement currently doesn't work anyway because the same field as SSA_NAME_DEF_STMT is reused for the free list chaining. There's a flag to tell me whether a node is in the free list, but of course that flag isn't set for the limbo nodes that you and I agree are a problem -- because they aren't in the free list (and they aren't in the program either). -- Robert
Re: SSA_NAMES: should there be an unused, un-free limbo?
> Right now we can have SSA_NAMEs in the > list which are no longer used, and we have no way to tell whether they > are used or not. Thus the only way to see all valid SSA_NAMEs is to > walk the code. To wit: are there iteration macros somewhere that will help me walk the code while abstracting away all the ugly details like stmt/bb boundaries, etc.? -- Robert
Re: SSA_NAMES: should there be an unused, un-free limbo?
> Robert, can you attach the testcase you've been working with? One testcase is libstdc++-v3/libsupc++/vec.cc from mainline. But it compiles without trouble unless you add verification or a walk over the SSA_NAMEs at the right time. > 1. We replace all uses of a phi node with something else > 2. We then call remove_phi_node with false as the last parameter (only > 3 places in the compiler), which ends up destroying the phi node but > not releasing the LHS name (since this is what the last parameter says > whether to do). That's right. Zdenek recommended that I change one of those places (the one in tree_merge_blocks that you quoted) to "true", but doing that causes some other code of his to break. I haven't analyzed why he needs those things to stay around, but I suspect it's because -- for a while, anyway -- he needs the DEF_STMT for something that's been removed. -- Robert
Re: SSA_NAMES: should there be an unused, un-free limbo?
> Something I forgot to add in my previous message. Notice that it is not > altogether rare to find cases where we have more SSA names than > statements. Are you walking the SSA names because you assume it's > always shorter than walking the statements? No. I'm walking the SSA names because logically that's what the algorithm is interested in. At that level, the algorithm doesn't care about statements. -- Robert
Re: SSA_NAMES: should there be an unused, un-free limbo?
> You can't change the last parameter to true in the if's true branch. Ouch. Duh. Thanks! The worst of it is, Zdenek sent me a patch, and not only did I misunderstand it, I then transcribed it wrong based on my misunderstanding. Oh well. Thanks for guessing my mistake. -- Robert