[PATCH] Clarify wording in the LangRef around !invariant.load

[Sanjoy Das]

Just to be clear: I have no problem with this change.  However, I
think the current semantics of invariant loads is counter-intuitive,
and should be clarified sometime in the future.

> This is definitely valid:
>
> int *p = <known dereferenceable>
> m1 = load(p) !invariant
> store(q)
> m2 = load(p)
> if (something)
>   foo(m1)
> else
>   bar(m2)

I claim that this IR is only conditionally well-defined -- the
semantics say that "once the location is known dereferenceable along a
particular execution path that it's value is henceforth unchanging".
If p == q, and the value being stored by store(q) is not the same as
what p originally had, the program is ill-defined.  But it was also
ill-defined before in that case, so the transform itself is valid.

> You would be forced to do this, which I think would prevent reordering store(q) and m1 = load(p):
>
> int *p = <known dereferenceable>
> store(q)
> if (something)
>   assume_invariant(p)
>   m1 = load(p) ;; ***
>   foo(m1)
> else
>   m2 = load(p)
>   bar(m2)


My argument is that the above is the only intuitive interpretation of an
invariant load (if the *** load was marked invariant).  With the
current semantics of invariant loads, the following transforms (which
I believe are all correct and intuitive) make a well-defined program
undefined:

%a = < known dereferenceable >
store 0, %a
store 1, %a
if ($false-at-runtime) {
  call undef(%a)  ;; well-defined, because it never executes
}

suitable choice for undef, and inlining ==>

%a = < known dereferenceable >
store 0, %a
store 1, %a
if ($false-at-runtime) {
  %b = load %a !invariant
  use(%b)
}

to ==>

%a = < known dereferenceable >
store 0, %a
%b = load %a !invariant
store 1, %a
if ($false-at-runtime) {
  use(%b) ;; this is still the original use undef(%b)
          ;; which initially was okay because it would never
          ;; execute
}

to ==>

%a = < known dereferenceable >
unreachable ;; modifying invariant location, "cannot happen"
            ;; since the original value at %a is either != 0
            ;; or != 1


As far as I can tell, there are three options for invariant_load:

  1. we treat invariant_loads as side-effecting operations.  this
     prevents hoisting through control flow.

  2. we accept this weird corner case of semantics not just following
     from what was actually executed, and teach the rest of the
     pipeline about it (so, for example, there are some values undef
     cannot be replaced by, and the first transform is declared
     illegal).

  3. define a mutable location that has invariant loads in a way that
     the last transform is not sound (i.e. locations that have
     invariant loads can be modified), but I cannot think of a way to
     do that in a sound way.


The other option is to introduce a notion of an invariant attribute
that we can put on function arguments and return values (just like
nonnull).  This lets us mark incoming arguments as invariant
locations, and "make_invariant" is a just another function that has
its return value tagged with the "invariant" attribute.  This can then
be used exactly as you and Philip have suggested on this and other
threads.  Once that is in place, we can make the definition of an
invariant_load weaker, sound and just a convenient notation for

int *p = <known dereferenceable>
store(q)
if (something)
  p1 = make_invariant(p)
  m1 = load(p1) ;; was m1 = load-invariant p
  foo(m1)
else
  m2 = load(p)
  bar(m2)

-- Sanjoy