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

[Andrew Trick]

On Nov 20, 2014, at 11:05 PM, Sanjoy Das <sanjoy at playingwithpointers.com> wrote:

>> This intrinsic would be control dependent, and only reads from its argument (I think).
> 
> This would get DCE'ed if it is marked as nounwind, so we'll have to
> figure out a way to get llvm to have readonly functions that can
> throw.  But in principle, I think this can be made to work.
> 
>> I claim that this sequence involving !invariant.load metadata:
>> ---
>> int *p = <known dereferenceable>
>> ...
>> <any arbitrary operations or control flow>
>> ...
>> m = load(p) !invariant.load
>> ---
>> Is equivalent to this sequence, given the definition of assume_attribute above:
>> ---
>> int *p = <known dereferenceable>
>> llvm_assume_attribute(p) invariant
>> ...
>> <any arbitrary operations or control flow>
>> ...
>> m = load(x)
> 
> Agreed.
> 
>> ---
>> The semantic hole arises because we could have two pointer definitions:
>> ---
>> int *p1 = <known dereferenceable>
>> ...
>> int *p2 = <known dereferenceable>
>> ...
>> <any arbitrary operations or control flow>
>> ...
>> if (p1 == p2)
>>  m = load(p2) !invariant.load
>> —
>> 
>> Substituting p1 for p2 in the load now changes the semantics.
> 
> This is not the semantic issue I was trying to get at, though they may
> be related in a way I don't see yet.  What I was trying to say was
> that we should be able to reason that adding "if ($false) { XXX; }" to
> a CFG, for any value of 'XXX' and any value of $false that will always
> be false should be legal and semantically irrelevant, because XXX is
> never executed.  However, in the current scheme for invariant loads,
> if XXX is an invariant load,
> adding this "never taken" control flow changes semantics.  This is not
> a problem in theory, or in an abstract sense (it is perfectly sound
> semantics, AFAICT); but practically it means we won't be able to
> reason about transforms on the IR as "we added this bit of LLVM IR,
> but it never executes so it doesn't matter" because things that don't
> execute do matter sometimes.
> 
> For example, if I have the IR:
> 
> %p = < known dereferenceable >
> if ($always-false-at-runtime) {
>  call undef(%p)
> }
> 
> I cannot replace undef with a function that has an invariant load on
> the first argument.

I understand. I was pointing out something totally unrelated. 

Regarding the issue you raise - most metadata and flags hold true for only for the original uses of the value (IIUC). For example, if a load with range is hoisted and CSEd with a normal load, the range would need to be dropped. 

What I'm proposing with invariant is a stronger condition. I think it is consistent with Philip's langref wording, but inconsistent with usual metadata rules.

I would actually be fine with more conservative semantics - the load is only invariant with respect to its original uses - because it won't affect optimization in practice.  But this won't decompose nicely as an assume-invariant intrinsic plus normal load. Assume-invariant ties control and memory dependence together. A conservative invariant load would be effectively control dependent but not memory dependent. 
Not the same thing as:

%p = < known dereferenceable >
Store // alias with %p
if ( something)
  assume_invariant(%p)
  Load(%p)

Andy

> 
> -- Sanjoy