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

fhahn abandoned this revision. fhahn added a comment. In D64128#1578916 , @fhahn wrote: > In D64128#1576391 , @rjmccall wrote: > > > I wouldn't favor adding something really obscure that was only useful for > >

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

arichardson added a comment. In D64128#1576391 , @rjmccall wrote: > I wouldn't favor adding something really obscure that was only useful for > clang, but I think builtins to set and clear mask bits while promising to > preserve object-reference

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

fhahn added a comment. In D64128#1576391 , @rjmccall wrote: > I wouldn't favor adding something really obscure that was only useful for > clang, but I think builtins to set and clear mask bits while promising to > preserve object-reference identity

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

hfinkel added a comment. > Do we have any policies against using clang-only builtins in the codebase? No, but obviously it will need to be protected with appropriate ifdefs and integrated in some maintainable fashion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

rjmccall added a comment. I wouldn't favor adding something really obscure that was only useful for clang, but I think builtins to set and clear mask bits while promising to preserve object-reference identity would be more generally useful for libraries. For example, there might be somewhere

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

fhahn added a comment. In 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

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

hfinkel added a comment. In 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

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

rjmccall added a comment. 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

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

hfinkel added a comment. In D64128#1570609 , @rjmccall wrote: > In D64128#1569836 , @hfinkel wrote: > > > In D64128#1569817 , @rjmccall > > wrote: > > > > > The

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

rjmccall added a comment. In D64128#1569836 , @hfinkel wrote: > In D64128#1569817 , @rjmccall wrote: > > > The pointer/integer conversion is "implementation-defined", but it's not > > totally unconstrained. C

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

fhahn added a comment. In D64128#1569836 , @hfinkel wrote: > In D64128#1569817 , @rjmccall wrote: > > > The pointer/integer conversion is "implementation-defined", but it's not > > totally unconstrained. C notes

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

fhahn added a comment. Thanks for the quick responses and the helpful comments. Thank you very much Hal, for summarizing the argument from previous discussions. My initial understanding indeed was that by generating ptrmask directly for C/C++ expressions, we can circumvent the issues that come

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

hfinkel added a comment. In D64128#1569817 , @rjmccall wrote: > The pointer/integer conversion is "implementation-defined", but it's not > totally unconstrained. C notes that "The mapping functions for converting a > pointer to an integer or an integer

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

rjmccall added a comment. The pointer/integer conversion is "implementation-defined", but it's not totally unconstrained. C notes that "The mapping functions for converting a pointer to an integer or an integer to a pointer are intended to be consistent with the addressing structure of the

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

hfinkel added a comment. In D64128#1569590 , @efriedma wrote: > > If they're all syntactically together like this, maybe that's safe? > > Having them together syntactically doesn't really help, I think; it might be > guarded by some code that does the

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

efriedma added a comment. > If they're all syntactically together like this, maybe that's safe? Having them together syntactically doesn't really help, I think; it might be guarded by some code that does the same conversion (and if you repeat the conversion, it has to produce the same result).

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

rjmccall added a comment. I agree with Eli that this isn't obviously a legal transformation. `llvm.ptrmask` appears to make semantic guarantees about e.g. the pointer after the mask referring to the same underlying object, which means we can only safely emit it when something about the source

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

hfinkel added a comment. In D64128#1568857 , @efriedma wrote: > I don't think this transform is valid, for the same reasons we don't do it in > IR optimizations. I believe that in the problematic cases we previously discussed (e.g., from

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

jfb added a comment. I'm not sure I understand all the implications, and why that would / wouldn't be valid. Should this be an builtin that can be called from C++ directly? Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2034 + (AllOnes <<

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

efriedma added a comment. I don't think this transform is valid, for the same reasons we don't do it in IR optimizations. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64128/new/ https://reviews.llvm.org/D64128

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

fhahn updated this revision to Diff 207761. fhahn marked 2 inline comments as done. fhahn added a comment. Use IgnoreParens(). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64128/new/ https://reviews.llvm.org/D64128 Files:

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

fhahn added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2025 + + auto *CE = dyn_cast(peelParens(BO->getLHS())); + if (!CE || CE->getCastKind() != CK_PointerToIntegral) xbolva00 wrote: > BO->getLHS()->IgnoreParens() ? That's very useful,

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

xbolva00 added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2025 + + auto *CE = dyn_cast(peelParens(BO->getLHS())); + if (!CE || CE->getCastKind() != CK_PointerToIntegral) BO->getLHS()->IgnoreParens() ? Repository: rG LLVM Github

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

fhahn updated this revision to Diff 207754. fhahn added a comment. Strip away llvm changes, use ABIAlignment for address space. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64128/new/ https://reviews.llvm.org/D64128 Files:

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

fhahn created this revision. fhahn added reviewers: jfb, rsmith, rjmccall, efriedma, hfinkel. Herald added subscribers: llvm-commits, dexonsmith, hiraditya. Herald added projects: clang, LLVM. I am not sure if this is the best way to do the matching and would appreciate any pointers for improving