[PATCH] D64128: [CodeGen] Generate llvm.ptrmask instead of inttoptr(and(ptrtoint, C)) if possible.

[Hal Finkel via Phabricator via cfe-commits]

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