theraven added inline comments.
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14323
+ Result = Builder.CreateIntrinsic(
+ Intrinsic::ptrmask, {Args.SrcType, SrcForMask->getType(),
Args.IntType},
+ {SrcForMask, NegatedMask}, nullptr, "aligned_result");
----------------
fhahn wrote:
> lebedev.ri wrote:
> > arichardson wrote:
> > > lebedev.ri wrote:
> > > > Is sufficient amount of passes, analyses know about this intrinsic?
> > > Good question. In the simple test cases that I looked at the code
> > > generation was equivalent.
> > >
> > > In our fork we still use ptrtoint+inttoptr since I implemented them
> > > before the new intrinsic existed. But since the ptrmask instrinsic exists
> > > now I thought I'd use it for upstreaming.
> > > I'll investigate if this results in worse codegen for more complex uses.
> > >
> > (TLDR: before producing it in more cases in clang, i think it should be
> > first ensured
> > that everything in middle-end is fully aware of said intrinsic. (i.e. using
> > it vs it's
> > exploded form results in no differences in final assembly on a sufficient
> > test coverage))
> > I'll investigate if this results in worse codegen for more complex uses.
>
> The findings would be interesting indeed.
>
> One thing to watch out for is that ptrmask should give better alias analysis
> results than ptrtoint/inttoptr. Also, IIRC some instcombine transformations
> for ptrtoint/inttoptr are not strictly valid according to the LangRef. Not
> sure if that changed yet.
There is currently an open review on the semantics of inttoptr / ptrtoint. The
current LangRef underspecifies it. On most architectures, both are bitcasts.
On CHERI (including ARM's Morello system), it is not safe to round trip a
pointer via an integer at the IR or machine-code level (in C, `[u]intptr_t` is
represented as `i8*` in the IR). A few architectures do some arithmetic to
cast between pointer and integer. The mid-level optimisers are slowly getting
better at avoiding the patterns that rely on the underspecified behaviour, but
removing this need for them would be a benefit.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71499/new/
https://reviews.llvm.org/D71499
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
