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

[Sanjoy Das via llvm-commits]

Hi Daniel,

Daniel Berlin wrote:
 > FWIW: I've pretty much not thought about it at all.
 > The main use case, as far as i remember, was to support specifying
 > invariantness that otherwise could not be inferred through analysis.
 > I believe the only use case in my part of the world (C++) is const
 > objects of various sorts, or write-once objects.
 >
 > Neither would be broken by this kind of merging.
 >
 > If we do think the above kind of merging is okay, we should mark
 > invariant.end readonly as well and teach a small set of things not to
 > hoist them/eat them, so that we gain the benefits of them not blocking
 > optimization either (for example, any readnone/readonly function that
 > uses invariants is still going to be marked readwrite due to the
 > invariant.end)

I meant to mention this in my previous mail, but we'd need some
special handling / logic to not DCE them away if we mark them as
readonly.

-- Sanjoy


 >
 >
 >        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.
 >
 >
 > Yes, that sounds right.
 >
 >
 >     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.
 >
 >
 >
 > yeah. I guess it would be interesting to know what real world uses for
 > this currently exist.
 >
 >
 >     [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.
 >
 >