[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

[Florian Hahn via Phabricator via cfe-commits]

fhahn 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");
----------------
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71499/new/

https://reviews.llvm.org/D71499