[PATCH] D136310: [SPIR-V] Add atomic_flag builtin implementation

[Ilia Diachkov via Phabricator via llvm-commits]

iliya-diyachkov added inline comments.


================
Comment at: llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp:634-647
+  Register MemSemanticsReg;
+  unsigned Semantics = SPIRV::MemorySemantics::SequentiallyConsistent;
+  if (Call->Arguments.size() >= 2) {
+    std::memory_order Order =
+        static_cast<std::memory_order>(getIConstVal(Call->Arguments[1], MRI));
+
+    Semantics =
----------------
It seems that this fragment is very similar to `MemSemanticsReg` evaluation in `buildAtomicRMWInst()`, so we could separate it into a static functionit and avoid the code duplication.



================
Comment at: llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp:649-652
+  if (Opcode == SPIRV::OpAtomicFlagClear)
+    assert((Semantics != SPIRV::MemorySemantics::Acquire &&
+            Semantics != SPIRV::MemorySemantics::AcquireRelease) &&
+           "Invalid memory order argument!");
----------------
Move the if condition to the assertion.


================
Comment at: llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp:654-664
+  Register ScopeRegister;
+  SPIRV::Scope::Scope Scope = SPIRV::Scope::Device;
+  if (Call->Arguments.size() >= 3) {
+    auto CLScope = static_cast<SPIRV::CLMemoryScope>(
+        getIConstVal(Call->Arguments[2], MRI));
+    Scope = getSPIRVScope(CLScope);
+    if (CLScope == static_cast<unsigned>(Scope))
----------------
This fragment also looks similar to getting `ScopeRegister` in `buildAtomicRMWInst()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136310