[PATCH] D23214: [MDA] Treat invariant.start as non-dependence

[Sanjoy Das via llvm-commits]

Hi Daniel,

Daniel Berlin wrote:
 > Well, no.
 > If invariant end was readonly, it would value forward past invariant
 > ends, and extend the range in the case it was like this:
 >
 > %a =invariant.start
 > ...
 > invariant.end(%a)
 >
 > <non-memory affecting instructions>
 >
 > %b=invariant.start
 > ...
 > invariant.end(%b)
 >
 > As long as there are no memory writing instructions in the middle,
 > things will believe the two invariant starts produce the same value, and
 > forward it :)

I have to think about it a bit more before I can commit to this, but
at least intuitively the above seems fine -- if there are no memory
clobbering instructions between the two ranges then they should be
merged.  This can even be a valuable optimization in cases where the
"<non-memory affecting instruction>" bits were hidden behind a call
that was later inlined.

The way I see it is that invariant_start and invariant_end denote the
boundaries of a region that can be assumed to not have stores to a
specific location[1].  To preserve this property we only need to avoid
stores floating into the region, and for that readonly semantics is
sufficient for non-atomic accesses (and potentially some kinds of
atomic accesses too).  Readonly semantics may even be stronger than we
need, since I think we can allow stores floating *out* of the region.

I'm not yet fully confident that the above semantics will hold under
pressure, since it fails the test of "can I write an implementation of
invariant_start and invariant_end that will give me the guarantees I
need?".  The best I can do with invariant_start and invariant_end as
specified is:

   void* invariant_start(iN* ptr) {
     cell = malloc(sizeof(iN));
     *(iN*)cell = ptr;
     return cell;
   }

   void invariant_end(void* cell, iN* ptr) {
     assert(*(iN*)cell == *ptr);
   }


which still allows the transform:

x = invariant_start(ptr)
int val = *ptr;
print(val);
invariant_end(x, ptr)

==>

x = invariant_start(ptr)
int val_backup = *ptr;
*ptr = 400;
*ptr = val_backup;
int val = *ptr;
print(val);
invariant_end(x, ptr)


That's not a clear problem (after all, intrinsics are allowed to have
special semantics that may not be "implementable" in IR), but it is a
"slightly red" flag.

[1]: An alternate framing would be: no stores to that specific
   location change the contained value.

-- Sanjoy

 >
 > GVN is not quite smart enough yet, but it's about to be.
 >
 > GCC even has testcases that we both eliminate and PRE pure/const
 > (readonly/readnone) calls.