[SPIR-V] Support memory(...) function attributes

[mpaszkowski]

Summary

Adds support for memory(...) function attributes in SPIR-V function control info lowering. More about the memory effects attributes changes can be found here: https://discourse.llvm.org/t/rfc-unify-memory-effect-attributes/65579.

[mpaszkowski]

This patch fixes the trivial-function-with-attributes.ll LIT test that is already present in the repository. Additionally, also the get_global_offset.ll test passes.

[arsenm]

llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp

62

ReadNone still exists? I thought it was removed. Why check it?

[arsenm]

llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp

62

Oh, readnone still exists but only applies to parameters now. There's no point in checking it on a function

[mpaszkowski]

@arsenm I have removed the checks for ReadNone and also for ReadOnly. Both are replaced with the new memory(...) attributes in functions. I will keep tests for the legacy attributes because we have compilers feeding old LLVM IR into the SPIR-V backend.

[nikic]

In D139133#3996834, @mpaszkowski wrote:

@arsenm I have removed the checks for ReadNone and also for ReadOnly. Both are replaced with the new memory(...) attributes in functions. I will keep tests for the legacy attributes because we have compilers feeding old LLVM IR into the SPIR-V backend.

The old attributes are converted into the new one during IR parsing, so that probably doesn't really test anything useful?

[mpaszkowski]

In D139133#3997203, @nikic wrote:

In D139133#3996834, @mpaszkowski wrote:

@arsenm I have removed the checks for ReadNone and also for ReadOnly. Both are replaced with the new memory(...) attributes in functions. I will keep tests for the legacy attributes because we have compilers feeding old LLVM IR into the SPIR-V backend.

The old attributes are converted into the new one during IR parsing, so that probably doesn't really test anything useful?

Are there any plans to remove the conversion in the long term? If there are/would be then the LIT tests would give us useful information why some runtime tests are failing without the need for debugging.

[iliya-diyachkov]

The patch looks good to me. Thanks, Michal!

[arsenm]

In D139133#3997323, @mpaszkowski wrote:

In D139133#3997203, @nikic wrote:

In D139133#3996834, @mpaszkowski wrote:

@arsenm I have removed the checks for ReadNone and also for ReadOnly. Both are replaced with the new memory(...) attributes in functions. I will keep tests for the legacy attributes because we have compilers feeding old LLVM IR into the SPIR-V backend.

The old attributes are converted into the new one during IR parsing, so that probably doesn't really test anything useful?

Are there any plans to remove the conversion in the long term? If there are/would be then the LIT tests would give us useful information why some runtime tests are failing without the need for debugging.

The tests you have here would break if the parsing stops recognizing it. There's no plus to keeping the legacy check here

llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp

 #include "SPIRV.h"
 #include "SPIRVBuiltins.h"
 #include "SPIRVGlobalRegistry.h"
 #include "SPIRVISelLowering.h"
 #include "SPIRVRegisterInfo.h"
 #include "SPIRVSubtarget.h"
 #include "SPIRVUtils.h"
 #include "llvm/CodeGen/FunctionLoweringInfo.h"
+#include "llvm/Support/ModRef.h"
 
 using namespace llvm;
 
 SPIRVCallLowering::SPIRVCallLowering(const SPIRVTargetLowering &TLI,
                                      SPIRVGlobalRegistry *GR)
     : CallLowering(&TLI), GR(GR) {}
 
 bool SPIRVCallLowering::lowerReturn(MachineIRBuilder &MIRBuilder,
return MIRBuilder.buildInstr(SPIRV::OpReturnValue)
                           *STI.getRegBankInfo());
   }
   MIRBuilder.buildInstr(SPIRV::OpReturn);
   return true;
 }
 
 // Based on the LLVM function attributes, get a SPIR-V FunctionControl.
 static uint32_t getFunctionControl(const Function &F) {
+  MemoryEffects MemEffects = F.getMemoryEffects();
+
   uint32_t FuncControl = static_cast<uint32_t>(SPIRV::FunctionControl::None);
-  if (F.hasFnAttribute(Attribute::AttrKind::AlwaysInline)) {
+
+  if (F.hasFnAttribute(Attribute::AttrKind::NoInline))
+    FuncControl |= static_cast<uint32_t>(SPIRV::FunctionControl::DontInline);
+  else if (F.hasFnAttribute(Attribute::AttrKind::AlwaysInline))
     FuncControl |= static_cast<uint32_t>(SPIRV::FunctionControl::Inline);
-  }
-  if (F.hasFnAttribute(Attribute::AttrKind::ReadNone)) {
+
+  if (MemEffects.doesNotAccessMemory())
     FuncControl |= static_cast<uint32_t>(SPIRV::FunctionControl::Pure);
-  }
-  if (F.hasFnAttribute(Attribute::AttrKind::ReadOnly)) {
+  else if (MemEffects.onlyReadsMemory())
     FuncControl |= static_cast<uint32_t>(SPIRV::FunctionControl::Const);
-  }
-  if (F.hasFnAttribute(Attribute::AttrKind::NoInline)) {
-    FuncControl |= static_cast<uint32_t>(SPIRV::FunctionControl::DontInline);
-  }
+
   return FuncControl;
 }
 
 static ConstantInt *getConstInt(MDNode *MD, unsigned NumOp) {
   if (MD->getNumOperands() > NumOp) {
     auto *CMeta = dyn_cast<ConstantAsMetadata>(MD->getOperand(NumOp));
     if (CMeta)
       return dyn_cast<ConstantInt>(CMeta->getValue());

llvm/test/CodeGen/SPIRV/function/trivial-function-with-attributes.ll

 ; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
 
 ;; FIXME: Are there any attributes that would make the IR invalid for SPIR-V?
 
 ;; Names:
 ; CHECK-DAG: OpName %[[#FN1:]] "fn1"
 ; CHECK-DAG: OpName %[[#FN2:]] "fn2"
 ; CHECK-DAG: OpName %[[#FN3:]] "fn3"
 ; CHECK-DAG: OpName %[[#FN4:]] "fn4"
 ; CHECK-DAG: OpName %[[#FN5:]] "fn5"
 ; CHECK-DAG: OpName %[[#FN6:]] "fn6"
 ; CHECK-DAG: OpName %[[#FN7:]] "fn7"
+; CHECK-DAG: OpName %[[#FN8:]] "fn8"
+; CHECK-DAG: OpName %[[#FN9:]] "fn9"
 
 ;; Types:
 ; CHECK:     %[[#VOID:]] = OpTypeVoid
 ; CHECK:     %[[#FN:]] = OpTypeFunction %[[#VOID]]
 
 
 ;; Functions:
 

 
 define void @fn5() readnone {
   ret void
 }
 ; CHECK: %[[#FN5]] = OpFunction %[[#VOID]] Pure %[[#FN]]
 ; CHECK: OpFunctionEnd
 
 
-define void @fn6() readonly {
+define void @fn6() memory(none) {
   ret void
 }
-; CHECK: %[[#FN6]] = OpFunction %[[#VOID]] Const %[[#FN]]
+; CHECK: %[[#FN6]] = OpFunction %[[#VOID]] Pure %[[#FN]]
 ; CHECK: OpFunctionEnd
 
 
-define void @fn7() alwaysinline readnone {
+define void @fn7() readonly {
   ret void
 }
-; CHECK: %[[#FN7]] = OpFunction %[[#VOID]] Inline|Pure %[[#FN]]
+; CHECK: %[[#FN7]] = OpFunction %[[#VOID]] Const %[[#FN]]
+; CHECK: OpFunctionEnd
+
+
+define void @fn8() memory(read) {
+  ret void
+}
+; CHECK: %[[#FN8]] = OpFunction %[[#VOID]] Const %[[#FN]]
+; CHECK: OpFunctionEnd
+
+
+define void @fn9() alwaysinline readnone {
+  ret void
+}
+; CHECK: %[[#FN9]] = OpFunction %[[#VOID]] Inline|Pure %[[#FN]]
 ; CHECK: OpFunctionEnd