[PATCH] D22793: IR: Introduce inbounds attribute on getelementptr indices.

[Peter Collingbourne via llvm-commits]

pcc added inline comments.

================
Comment at: docs/LangRef.rst:7548
@@ +7547,3 @@
+comparison is undefined if either operand was derived from an ``inrange``
+GEP, with the exception of comparisons where both operands were derived from
+a GEP which was evaluated with identical operands up to and including the
----------------
hfinkel wrote:
> eli.friedman wrote:
> > pcc wrote:
> > > hfinkel wrote:
> > > > eli.friedman wrote:
> > > > > Hmm... this sort of restricts the possible uses of the marking.  For example, given:
> > > > > 
> > > > >     struct X {
> > > > >         int x, y[2];
> > > > >     };
> > > > >     int f(struct X *x, int i) { return x->y[i]; }
> > > > >     int *f2(struct X *x) { return x->y; }
> > > > > 
> > > > > We can mark the GEP in `x->y[i]` inrange, but we can't mark the GEP in `x->y` inrange because it could be used in a pointer comparison.  I guess that's okay (the stated form is convenient for performing SROA).  That said, inevitably someone is going to push for a version which doesn't make comparisons undefined so it can be used more aggressively in C code.
> > > > > 
> > > > > You should probably state explicitly that the result of converting an inrange pointer to an integer is undef; you can't actually do anything useful with the result anyway.
> > > > What's the motivation for the current language? I don't think we can have anything that strict.
> > > The intent was to allow optimization passes to re-arrange elements of an indexed object (i.e. what SplitGlobals does). Although at this point it does seem that trying to restrict pointer comparisons in general would be likely to make this too restrictive for use by C family frontends. Instead I think the best approach would be to change SplitGlobals to check for the unnamed_addr attribute, and remove the pointer comparison language here.
> > > 
> > > > You should probably state explicitly that the result of converting an inrange pointer to an integer is undef;
> > > 
> > > Will do.
> > The motivation here is that we want to be able to say "if every use of a given global is an inrange GEP, SROA is legal (i.e. we can split the global)".  That requires some sort of restriction on comparisons because we could change the result of a comparison in an unpredictable way.
> I'd think that we just need to check the use-def chains, starting with uses of the global, to see if the address was captured.
The problem with that, at least for vtables, is that the vtable address will be captured in most normal cases (e.g. the constructor will store the vtable address in the object's vtable pointer). So for split globals I think we need to rely on unnamed_addr here.


https://reviews.llvm.org/D22793