hfinkel added a comment. In D64128#1572504 <https://reviews.llvm.org/D64128#1572504>, @rjmccall wrote:
> I would be opposed to any addition of a `-f` flag for this absent any > evidence that it's valuable for some actual C code; this patch appears to be > purely speculative. I certainly don't think we should be adding it > default-on. Your argument about alignment is what I was referring to when I > mentioned that this is enforcing alignment restrictions in places we > traditionally and intentionally haven't. My underlying thought here is: The more we generate a particular IR construct the more quickly we'll find the bugs and the better we'll end up optimizing it. Thus, I found the idea behind this patch appealing. My experience is also that providing more pointer underlying-object information tends to help code quality. However, I definitely see your point that this is potentially risky: bugs uncovered by generating this intrinsic based on this code pattern might reveal only code doing interesting things with pointer bits and not actual optimizer bugs. I think that we have different feelings on the magnitude of that risk, but I definitely glad that you weighed in here. I suggest that we drop this unless and until we have benchmark data showing it to be useful. If we do, then we can consider an off-by-default flag. It will also be interesting to explore adding a Clang intrinsic. > Note that it won't help Clang's major uses of `PointerIntPair` and > `PointerUnion` because our traits say that types like `Decl` and `Type` have > more alignment bits than their formal alignment would indicate, and > `PointerIntPair` occupies alignment bits starting with the most significant. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64128/new/ https://reviews.llvm.org/D64128 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
