dexonsmith added a comment. Oh, de-refining is pretty nifty / evil. This patch has background: https://reviews.llvm.org/D18634
Since 2016, the optimizer is not allowed to do IPA on functions that can be de-refined (such as `linkonce_odr` functions). Here's a hypothetical problematic scenario for the optimizer: - original IR for B has a `throw` somewhere - function A invokes function B - in this TU, B is optimized and removes exceptions, and gets marked `nounwind` - function A leverages the `nounwind` to turn the `invoke` into a `call` - function B is de-refined at link/load time: linker chooses a *different* function B which still has a `throw` - "evil magic" happens (see the discussions around the patch linked above) - a crash is introduced At first blush, it sounds like this could only be a problem if the code has UB in it. However, read the above patch (and follow-ups, and related discussion) for a variety of examples of non-UB cases where IPA on de-refineable functions introduces crashes. I don't know for sure this could be a problem for `nounwind` specifically, but in general the LLVM optimizer doesn't look at attributes of de-refineable functions. (Note that if you're doing LTO (like AutoFDO), this usually won't block optimization, since at LTO time there are very few de-refineable functions (most `linkonce_odr` functions are internalized, and not exported as weak). So if we added a `-cc1` flag to prefer "stable IR" over "frontend peepholes", it would make sense for `-flto` to imply it.) On the other hand, the frontend knows the token sequence from the source language. It knows whether function `B` is inherently `nounwind` based on its ODR token sequence; in which case, it's safe to use the attribute for an IPA peephole. ---- BTW, I'm not personally against removing this peephole from the frontend (even without a flag), or limiting it somehow to cases where it doesn't make IR output unstable. I like the idea of stable IRGen output. Nevertheless, it feels like removing IPA-based peepholes from Clang in the name of stable IRGen output is a change in direction, which might deserve a discussion in the forums rather than in a particular patch review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83906/new/ https://reviews.llvm.org/D83906 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
