[PATCH] D21255: Fix cloning GlobalValues with comdats across Modules.
Justin Lebar via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 15 10:33:25 PDT 2016
jlebar added a comment.
In http://reviews.llvm.org/D21255#458782, @rnk wrote:
> Anything wrong with forcing the cross-module use case to be a little more complicated?
I guess the suggestion is,
- leave the implementation of copyAttributesFrom alone, and
- add a comment explaining what needs to be done if you're copying between modules?
I guess I'm fine with that, although given that we already had a nontrivial bug (that took me half a day to track down), I am a bit concerned about leaving the footgun there as-is. Even if all of the code in llvm is correct, this seems like a pretty sharp edge to expose to future authors.
What if we modified Function::setParent to either
- assert that the comdat is from the right module, or
- fix up the comdat?
I guess if cross-module copiers have to fix up the initializers, an assertion might be the most appropriate thing, but I'm fine with either.
More information about the llvm-commits