[PATCH] D80319: [LICM] Allow movement of non-aliasing calls that at most only write to memory

[Dominic Chen via Phabricator via llvm-commits]

ddcc added a comment.

Thanks for the feedback!

> If `@foo2e` writes 2 to str, and `@foo2f` reads, increments by 1 and writes back, and the loop runs 3 times.
>  Code below gives result 3. When `@foo2e` is hoisted, the result is 5.
>  At the very least you need to check `pointerInvalidatedByLoop` for all call args.

I am missing a check on noalias arguments, but wouldn't it need to be for reference instead of modification (as implemented by `pointerInvalidatedByLoop*`)? For example, if the increment call (`foo2f`) occurred before the write call (`foo2e`), then it is no longer correct to hoist out the write either. I'm also not sure what to do with the MemorySSA variant when the AliasSetTracker is not available; MemorySSA seems to generate only a single `MemoryDef` for a call, which doesn't distinguish between definitions of arguments or other memory.

Instead, perhaps it'd be easier if I just limited this case to `inaccessiblememonly` calls with `readnone` arguments? That is my primary use case, anyway.

> For inaccessible memory, if two of the calls inside the loop write to that same inaccessible memory location, and one meets the condition to hoist, while the other also reads, then it's legal to hoist one of them?
>  `@foo2e` always writes 2 to that location.
>  `@foo2f` reads that location, increments by 1 and writes it back.
>  With an iteration count of 3, the original code will hold 3 at that location. After hoisting it will hold 5.

That's a good point. The inaccessible memory that I'm interested in is memory-mapped from a hardware peripheral, so I know it doesn't alias, but I don't see a good way to express that with the existing function attributes. Even with LTO, there's no guarantee that an `inaccessiblememonly` function can't alias against another regular external function. The closest relevant attribute would be `noalias`, but that only applies to memory derived from input arguments, which would mean nothing on a `readnone` argument. I suppose what I'm looking for is some sort of `exclusive` attribute, which would indicate that the inaccessible memory can't be accessed in any other way.

Another potential issue is whether `writeonly` must be deterministic? `readonly` is guaranteed to be pure, but `writeonly` is only guaranteed to not read from memory or have other side-effects. For example, can a `writeonly` function access the timestamp counter and write only if the result is even?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80319/new/

https://reviews.llvm.org/D80319