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

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In D71499#1801421 <https://reviews.llvm.org/D71499#1801421>, @lebedev.ri wrote:

> (temporarily undoing my review since it looks like things may change)
>
> In D71499#1801302 <https://reviews.llvm.org/D71499#1801302>, @arichardson wrote:
>
> > ...
>
>
> Aha! :)
>
> In D71499#1801302 <https://reviews.llvm.org/D71499#1801302>, @arichardson wrote:
>
> > However, I think this means that for the downstream CHERI codegen
> >  there is one more case where we need to be careful
> >  (but that shouldn't matter for upstream).
> >  For us `uintptr_t` is represented as `i8 addrspace(200)*`.
> >  As `uintptr_t` can hold values that are plain integers and not pointers,
> >  we would need to add the inbounds only if the AST type is a pointer,
> >  and use a non-inbounds GEP for `(u)intptr_t`.
>
>
> I may be missing some details, but won't we get that for free,
>  since we only do `gep` for pointers already?


I believe we would need something like this downstream instead of always creating an inbounds GEP:

  if (E->getType()->isPointerType()) {
    // The result must point to the same underlying allocation. This means we
    // can use an inbounds GEP to enable better optimization.
    Result = Builder.CreateInBoundsGEP(EmitCastToVoidPtr(Args.Src),
                                       Difference, "aligned_result");
  } else {
    // However, for when performing operations on __intcap_t (which is also
    // a pointer type in LLVM IR), we cannot set the inbounds flag as the
    // result could be either an arbitrary integer value or a valid pointer.
    // Setting the inbounds flag for the arbitrary integer case is not safe.
    assert(E->getType()->isIntCapType());
    Result = Builder.CreateGEP(EmitCastToVoidPtr(Args.Src), Difference,
                               "aligned_result");
  }



> In D71499#1801302 <https://reviews.llvm.org/D71499#1801302>, @arichardson wrote:
> 
>> Does that sound correct or did I misunderstand something?
> 
> 
> To //me// this all sounds great.
>  Since we get to dictate^W define semantics, i'd think we can require
> 
> In D71499#1801302 <https://reviews.llvm.org/D71499#1801302>, @arichardson wrote:
> 
>> both input and output should be a `Logical pointer`.
>>  If you want random values, you should be aligning uintptr_t instead.

Thank you very much for the useful suggestion!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499