Hi, On Sun, 12 Jul 2015, Tom de Vries wrote:
> > I'm trying to get to a defined policy for what is allowed for caches. > > Either forbidding or allowing multi-step dependencies, I don't really > > mind. I think forbidding is the way to go, because ... > > I managed to write a patch series that implements the forbidding of > > multi-step dependencies. I'll post this soon. > > https://gcc.gnu.org/ml/gcc-patches/2015-07/msg00970.html ... aha, it finds bugs! So you actually had to changes some hashes to be non-caching for this to work, and it's all some decl-to-debug-something maps (well, of course, otherwise you wouldn't have run into the bug you're trying to fix in the first place). I think this hints at actual bugs that needs fixing in the gomp branch: As you analyzed in PR 66714, eventually a decl A is replaced by decl B, but its debug-expr is simply copied, and that one itself refers to decls (let's call them D*) that meanwhile are removed. Now, as the D* decls are not in any other data structure (otherwise they would have been marked) the typical actions that needed to have been done for them (like e.g. associating debug info with them, allocating them to some stack or register place) i.e. anything that needed to be done for normal decls won't have been done. So the debug info generator in this case, when it sees those D* decls can't do its work, e.g. debug info generated for D* won't refer to the real place containing the value, because also the generated code itself doesn't refer to D* anymore. This also hints at other problems (which might not actually occur in the case at hand, but still): the contents of DECL_VALUE_EXPR is the "real" thing containing the value of a decl (i.e. a decl having a value-expr doesn't itself occur in the code anymore), be it a decl itself, or some expression (which might also refer to decls). Now, in PR 66714 you analyzed that one of those D* was removed from the function, which should have happened only because no code referred to anymore, i.e. D* was also rewritten to some other D'* (if it weren't rewritten and D* was referred to in code, you would have created a miscompilation). At that point also the DECL_VALUE_EXPRs need to be rewritten to refer to D'*, not to D* anymore. Implementing multi-step maps or making the hashmaps non-caching doesn't solve any of the above problems, it merely forces some DECLs in the compiler to remain live but that actually have no meaning in their context. So, I think this makes it pretty clear that those hashmaps should remain caching maps, and that multi-step deps in caches should be disallowed, and that the underlying problem should rather be fixed (and the checking code against multi-step-deps should be added to the compiler). Ciao, Michael.
