[Mlir-commits] [clang] [llvm] [mlir] Extend MemoryEffects to Support Target-Specific Memory Locations (PR #148650)

[Nikita Popov]

nikic wrote:

> Of which, I've being pondering the bitcode issue for a week or so and have yet to convince myself there's a migration path for existing intrinsics (because a function may have inferred its memory affect based on its contents). As a minimum, given the context of the target memory location being target specific it means the bitcode handling also becomes target specific? However, even the worse case scenario of only being able to use the new locations for new intrinsics can work, I'd just rather be able to migrate the existing ones.

I'd expect the bitcode upgrade to work by splatting the value of the current inaccessiblemem location into the target memory locations. That should be conservatively correct.

My proposed hierarchy would also implicity result in the same behavior for textual IR files (as the old `inaccessiblemem: foo` will be equivalent to `other_inaccessiblemem: foo, target_mem0: foo, target_mem1: foo, ...`).

> I was thinking the following can be done independently:
> 
>   1. Improve alias analysis so it better disambiguates between target memory locations.
>   2. Implement the location hierarchy you propose.
>   3. Improve the IR printing so that it scales better as the number of inaccessible/target locations increases.
>   4. Migrate intrinsic definitions to use IntrRead/IntrWrite and remove the more specialised options. NFC change.
>   5. Add target specific pretty printing and parsing support.

Sounds reasonable to me. Just need to be careful to not use the target memory locations outside of tests before everything is in place.

https://github.com/llvm/llvm-project/pull/148650