[PATCH] D64414: Do not set ReadNone attribute on intrinsics with side effects

[Momchil Velikov via Phabricator via llvm-commits]

chill added a comment.

In D64414#1576588 <https://reviews.llvm.org/D64414#1576588>, @jdoerfert wrote:

> In D64414#1576554 <https://reviews.llvm.org/D64414#1576554>, @chill wrote:
>
> > In D64414#1576252 <https://reviews.llvm.org/D64414#1576252>, @jdoerfert wrote:
> >
> > > I'm a bit lost. What intrinsic do we have that is "no-mem" but has side-effects?
> >
> >
> > E.g  `__builtin_arm_tcommit`, which I'm adding in this series, or  `__builtin_arm_irg` (which is *should* be `NoMem`, but is not, perhaps as a workaround for this issue).
>
>
> (Sorry if I'm way off here but I'd like to understand this better)
>
> `tcommit` makes the transaction state visible, correct? If so, I'd argue it modifies the memory (especially for other threads). Maybe my confusion stems from the IR vs MIR conflict here but in IR, I would not like `tcommit` to be `readnone` for multiple reasons, including memory movement around it.


`tcommit` does not modify memory (at least in my understanding what "modify memory" means), in the sense that if I do:

  *p = x;
  tcommit();
  z = *p

I would expect `x == z`, i.e the above to be equivalent to

  *p = x;
  tcommit();
  z = x;

Definitely we wouldn't like memory operations to be moved around transactional primitives.


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

https://reviews.llvm.org/D64414