[PATCH] D77213: Handle part-word LL/SC in atomic expansion pass

[James Y Knight via Phabricator via llvm-commits]

jyknight added a comment.

My first reaction is to be a bit reticent to extend the IR-level LL/SC expansion code, because generally speaking LL/SC expansion in IR is not sound (per previous discussions over the years).  However, I do recall during those discussions, it was also mentioned that the concerns don't apply to Hexagon [http://lists.llvm.org/pipermail/llvm-dev/2018-June/124069.html] -- that the activity of other simultaneously running threads cannot cause the spurious loss of a reservation. Perhaps that's the case now because there aren't any Hexagon chips that support multiple cores with separate caches.

So, this seems okay in concept, even though I do think it would be preferable if everyone followed the model of doing the expansion late in MI, rather than IR.



================
Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:1350-1352
+  // The newly inserted PHI nodes may be unnecessary, for example, they
+  // may have only one argument. Simplify the code eliminating those
+  // PHI that can be easily removed. Since the "exit value" (LoadedFull)
----------------
What's all this complication about?

I find it hard to imagine putting this kind of simplification code inline here is the best way to go about this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77213