paulkirth added a comment.
In D131306#3756009 <https://reviews.llvm.org/D131306#3756009>, @tejohnson wrote: > @davidxl @xur for review and thoughts. > > I'm a little wary of requiring that both pieces of metadata be carried > together, as that seems very brittle to maintain in the compiler. What would > happen if the MD_expected didn't get propagated by some pass along with the > MD_prof? I think you would get a false negative, which I suppose is better > than a false positive. An alternative, that I guess would require more > extensive changes, is to add an additional item to the "branch_weights" list > (would need to be obviously distinguishable by type from the list of weights > since that can be variable though). Agreed. This isn't my preferred solution, but it seemed far less invasive than changing the format of profiling metadata. Originally, I had looked into adding a provenance field to the metadata. It required changes to every test that has branch weights, and I balked at submitting that. I'm also a little wary of making a heavily used metadata type larger. I guess there isn't a lot of difference, but the external metadata is optional, and we could remove it after checking is complete. Unfortunately, I just don't think there is a clean solution here. Either we make an invasive change to the metadata format, or we deal w/ updating 2 pieces of metadata everywhere. I'm just very unsure about which is the right tradeoff. > Patch needs tests showing uses of the new metadata, and some documentation in > LangRef (i.e. near https://llvm.org/docs/LangRef.html#prof-metadata). Yes, testing and documentation is something I plan to improve, but I wanted to get some feedback on this approach before investing too heavily. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131306/new/ https://reviews.llvm.org/D131306 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
