[PATCH] D92842: [SelectionDAG] Add Target-Independent Compiler Barrier

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

Replying here mostly so I can find the information later.

Over the last few days, I have submitted a couple of changes which factor out a target independent MEMBARRIER node.  At this point, all targets except ARM, AArch64, AMDGPU, and WebAssembly have been migrated.  The former two are on review, the third I don't plan to touch as it uses a late expansion of atomic_fence instead, and has some target specific logic based on that which is hard to follow without architecture context I don't have.  The fourth I haven't looked at closely as I just found the alternate spelling of the barrier name it uses.

The choice of MEMBARRIER vs COMPILER_BARRIER naming was arbitrary.  I went with what more targets seemed to have used to minimize test churn.

I didn't find an in tree example which required the ordering on the generic instruction.  Given that, I left it out.  I am not opposed to adding it, but would like to see a justification before we add complexity.

As far as I can tell, the scope argument in this patch is purely spurious.  A MEMBARRIER can only have a single scope - SingleThread.  This is definitely true for all the targets I've looked at closely.  AMDGPU *might* be a counter example, but I don't understand the semantics of it's scopes closely well enough to know for sure.

Finally, this patch contains two functional changes to the x86 backend.  I did not include those.  Separating those out - with tests! - would likely be valuable.  I'd be happy to review if you wanted to do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92842