Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Translate nonsemantic attribute and metadata of GlobalVariable #2944

Merged
merged 4 commits into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 36 additions & 19 deletions lib/SPIRV/SPIRVReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5141,36 +5141,53 @@ void SPIRVToLLVM::transAuxDataInst(SPIRVExtInst *BC) {
return;
auto Args = BC->getArguments();
// Args 0 and 1 are common between attributes and metadata.
// 0 is the function, 1 is the name of the attribute/metadata as a string
auto *SpvFcn = BC->getModule()->getValue(Args[0]);
auto *F = static_cast<Function *>(getTranslatedValue(SpvFcn));
assert(F && "Function should already have been translated!");
// 0 is the global object, 1 is the name of the attribute/metadata as a string
auto *Arg0 = BC->getModule()->getValue(Args[0]);
auto *GO = cast<GlobalObject>(getTranslatedValue(Arg0));
auto *F = dyn_cast_or_null<Function>(GO);
auto *GV = dyn_cast_or_null<GlobalVariable>(GO);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes me a little bit suspicious, why do we need dyn_cast**_or_null** here when GO is a result of a cast? May be it's leftover from a previous patch iteration, where some test was failing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, yet it is a misuse.

assert((F || GV) && "Value should already have been translated!");
auto AttrOrMDName = BC->getModule()->get<SPIRVString>(Args[1])->getStr();
switch (BC->getExtOp()) {
case NonSemanticAuxData::FunctionAttribute: {
case NonSemanticAuxData::FunctionAttribute:
case NonSemanticAuxData::GlobalVariableAttribute: {
assert(Args.size() < 4 && "Unexpected FunctionAttribute Args");
// If this attr was specially handled and added elsewhere, skip it.
Attribute::AttrKind AsKind = Attribute::getAttrKindFromName(AttrOrMDName);
if (AsKind != Attribute::None && F->hasFnAttribute(AsKind))
return;
if (AsKind == Attribute::None && F->hasFnAttribute(AttrOrMDName))
return;
if (AsKind != Attribute::None)
if ((F && F->hasFnAttribute(AsKind)) || (GV && GV->hasAttribute(AsKind)))
return;
if (AsKind == Attribute::None)
if ((F && F->hasFnAttribute(AttrOrMDName)) ||
(GV && GV->hasAttribute(AttrOrMDName)))
return;
// For attributes, arg 2 is the attribute value as a string, which may not
// exist.
if (Args.size() == 3) {
auto AttrValue = BC->getModule()->get<SPIRVString>(Args[2])->getStr();
F->addFnAttr(AttrOrMDName, AttrValue);
} else {
if (AsKind != Attribute::None)
F->addFnAttr(AsKind);
if (F)
F->addFnAttr(AttrOrMDName, AttrValue);
else
F->addFnAttr(AttrOrMDName);
GV->addAttribute(AttrOrMDName, AttrValue);
} else {
if (AsKind != Attribute::None) {
if (F)
F->addFnAttr(AsKind);
else
GV->addAttribute(AsKind);
} else {
if (F)
F->addFnAttr(AttrOrMDName);
else
GV->addAttribute(AttrOrMDName);
}
}
break;
}
case NonSemanticAuxData::FunctionMetadata: {
case NonSemanticAuxData::FunctionMetadata:
case NonSemanticAuxData::GlobalVariableMetadata: {
// If this metadata was specially handled and added elsewhere, skip it.
if (F->hasMetadata(AttrOrMDName))
if (GO->hasMetadata(AttrOrMDName))
return;
SmallVector<Metadata *> MetadataArgs;
// Process the metadata values.
Expand All @@ -5180,14 +5197,14 @@ void SPIRVToLLVM::transAuxDataInst(SPIRVExtInst *BC) {
if (Arg->getOpCode() == OpString) {
auto *ArgAsStr = static_cast<SPIRVString *>(Arg);
MetadataArgs.push_back(
MDString::get(F->getContext(), ArgAsStr->getStr()));
MDString::get(GO->getContext(), ArgAsStr->getStr()));
} else {
auto *ArgAsVal = static_cast<SPIRVValue *>(Arg);
auto *TranslatedMD = transValue(ArgAsVal, F, nullptr);
auto *TranslatedMD = transValue(ArgAsVal, nullptr, nullptr);
MetadataArgs.push_back(ValueAsMetadata::get(TranslatedMD));
}
}
F->setMetadata(AttrOrMDName, MDNode::get(*Context, MetadataArgs));
GO->setMetadata(AttrOrMDName, MDNode::get(*Context, MetadataArgs));
break;
}
default:
Expand Down
41 changes: 25 additions & 16 deletions lib/SPIRV/SPIRVWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1230,23 +1230,27 @@ void LLVMToSPIRVBase::transFunctionMetadataAsUserSemanticDecoration(
}
}

void LLVMToSPIRVBase::transAuxDataInst(SPIRVFunction *BF, Function *F) {
auto *BM = BF->getModule();
void LLVMToSPIRVBase::transAuxDataInst(SPIRVValue *BV, Value *V) {
auto *GO = cast<GlobalObject>(V);
auto *F = dyn_cast_or_null<Function>(GO);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question about _or_null

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

auto *GV = dyn_cast_or_null<GlobalVariable>(GO);
assert((F || GV) && "Invalid value type");
auto *BM = BV->getModule();
if (!BM->preserveAuxData())
return;
if (!BM->isAllowedToUseVersion(VersionNumber::SPIRV_1_6))
BM->addExtension(SPIRV::ExtensionID::SPV_KHR_non_semantic_info);
else
BM->setMinSPIRVVersion(VersionNumber::SPIRV_1_6);
const auto &FnAttrs = F->getAttributes().getFnAttrs();
for (const auto &Attr : FnAttrs) {
const auto &Attrs = F ? F->getAttributes().getFnAttrs() : GV->getAttributes();
for (const auto &Attr : Attrs) {
std::vector<SPIRVWord> Ops;
Ops.push_back(BF->getId());
Ops.push_back(BV->getId());
if (Attr.isStringAttribute()) {
// Format for String attributes is:
// NonSemanticAuxDataFunctionAttribute Fcn AttrName AttrValue
// NonSemanticAuxData*Attribute ValueName AttrName AttrValue
// or, if no value:
// NonSemanticAuxDataFunctionAttribute Fcn AttrName
// NonSemanticAuxData*Attribute ValueName AttrName
//
// AttrName and AttrValue are always Strings
StringRef AttrKind = Attr.getKindAsString();
Expand All @@ -1259,19 +1263,20 @@ void LLVMToSPIRVBase::transAuxDataInst(SPIRVFunction *BF, Function *F) {
}
} else {
// Format for other types is:
// NonSemanticAuxDataFunctionAttribute Fcn AttrStr
// NonSemanticAuxData*Attribute ValueName AttrStr
// AttrStr is always a String.
std::string AttrStr = Attr.getAsString();
auto *AttrSpvString = BM->getString(AttrStr);
Ops.push_back(AttrSpvString->getId());
}
BM->addAuxData(NonSemanticAuxData::FunctionAttribute,
transType(Type::getVoidTy(F->getContext())), Ops);
BM->addAuxData(F ? NonSemanticAuxData::FunctionAttribute
: NonSemanticAuxData::GlobalVariableAttribute,
transType(Type::getVoidTy(V->getContext())), Ops);
}
SmallVector<std::pair<unsigned, MDNode *>> AllMD;
SmallVector<StringRef> MDNames;
F->getContext().getMDKindNames(MDNames);
F->getAllMetadata(AllMD);
V->getContext().getMDKindNames(MDNames);
GO->getAllMetadata(AllMD);
for (const auto &MD : AllMD) {
std::string MDName = MDNames[MD.first].str();

Expand All @@ -1284,11 +1289,11 @@ void LLVMToSPIRVBase::transAuxDataInst(SPIRVFunction *BF, Function *F) {
continue;

// Format for metadata is:
// NonSemanticAuxDataFunctionMetadata Fcn MDName MDVals...
// NonSemanticAuxData*Metadata ValueName MDName MDVals...
// MDName is always a String, MDVals have different types as explained
// below. Also note this instruction has a variable number of operands
std::vector<SPIRVWord> Ops;
Ops.push_back(BF->getId());
Ops.push_back(BV->getId());
Ops.push_back(BM->getString(MDName)->getId());
for (unsigned int OpIdx = 0; OpIdx < MD.second->getNumOperands(); OpIdx++) {
const auto &CurOp = MD.second->getOperand(OpIdx);
Expand All @@ -1304,8 +1309,9 @@ void LLVMToSPIRVBase::transAuxDataInst(SPIRVFunction *BF, Function *F) {
assert(false && "Unsupported metadata type");
}
}
BM->addAuxData(NonSemanticAuxData::FunctionMetadata,
transType(Type::getVoidTy(F->getContext())), Ops);
BM->addAuxData(F ? NonSemanticAuxData::FunctionMetadata
: NonSemanticAuxData::GlobalVariableMetadata,
transType(Type::getVoidTy(V->getContext())), Ops);
}
}

Expand Down Expand Up @@ -2028,6 +2034,7 @@ LLVMToSPIRVBase::transValueWithoutDecoration(Value *V, SPIRVBasicBlock *BB,
if (ST && ST->hasName() && isSPIRVConstantName(ST->getName())) {
auto *BV = transConstant(Init);
assert(BV);
transAuxDataInst(BV, V);
return mapValue(V, BV);
}
if (isa_and_nonnull<ConstantExpr>(Init)) {
Expand Down Expand Up @@ -2127,6 +2134,8 @@ LLVMToSPIRVBase::transValueWithoutDecoration(Value *V, SPIRVBasicBlock *BB,
GV->getAttribute(kVCMetadata::VCSingleElementVector), BVar);
}

transAuxDataInst(BVar, V);

mapValue(V, BVar);
spv::BuiltIn Builtin = spv::BuiltInPosition;
if (!GV->hasName() || !getSPIRVBuiltin(GV->getName().str(), Builtin))
Expand Down
2 changes: 1 addition & 1 deletion lib/SPIRV/SPIRVWriter.h
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ class LLVMToSPIRVBase : protected BuiltinCallHelper {
void transFunctionMetadataAsExecutionMode(SPIRVFunction *BF, Function *F);
void transFunctionMetadataAsUserSemanticDecoration(SPIRVFunction *BF,
Function *F);
void transAuxDataInst(SPIRVFunction *BF, Function *F);
void transAuxDataInst(SPIRVValue *BV, Value *V);

bool transGlobalVariables();

Expand Down
4 changes: 3 additions & 1 deletion lib/SPIRV/libSPIRV/NonSemantic.AuxData.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ namespace NonSemanticAuxData {
enum Instruction {
FunctionMetadata = 0,
FunctionAttribute = 1,
PreserveCount = 2
GlobalVariableMetadata = 2,
GlobalVariableAttribute = 3,
PreserveCount = 4
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If PreserveCount is (and has been previously) unused it should be probably deleted entirely. Otherwise, this line introduces a concern about binary incompatibility due to changing PreserveCount's value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deleted. It is unused.

};
} // namespace NonSemanticAuxData
4 changes: 4 additions & 0 deletions lib/SPIRV/libSPIRV/SPIRVExtInst.h
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,10 @@ inline void SPIRVMap<NonSemanticAuxDataOpKind, std::string>::init() {
"NonSemanticAuxDataFunctionMetadata");
add(NonSemanticAuxData::FunctionAttribute,
"NonSemanticAuxDataFunctionAttribute");
add(NonSemanticAuxData::GlobalVariableMetadata,
"NonSemanticAuxDataGlobalVariableMetadata");
add(NonSemanticAuxData::GlobalVariableAttribute,
"NonSemanticAuxDataGlobalVariableAttribute");
}
SPIRV_DEF_NAMEMAP(NonSemanticAuxDataOpKind, NonSemanticAuxDataOpMap)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
; RUN: llvm-as < %s -o %t.bc
; RUN: not llvm-spirv %t.bc -spirv-text --spirv-preserve-auxdata --spirv-max-version=1.5 --spirv-ext=-SPV_KHR_non_semantic_info,+SPV_INTEL_global_variable_decorations -o - 2>&1 | FileCheck %s --check-prefix=CHECK-SPIRV-EXT-DISABLED
; RUN: llvm-spirv %t.bc -o %t.spv --spirv-preserve-auxdata --spirv-max-version=1.5 --spirv-ext=+SPV_INTEL_global_variable_decorations
; RUN: llvm-spirv %t.spv -to-text -o - | FileCheck %s --check-prefixes=CHECK-SPIRV,CHECK-SPIRV-EXT
; RUN: llvm-spirv -r --spirv-preserve-auxdata %t.spv -o %t.rev.bc
; RUN: llvm-dis %t.rev.bc -o - | FileCheck %s --check-prefix=CHECK-LLVM
; RUN: llvm-spirv -r %t.spv -o %t.rev.without.bc
; RUN: llvm-dis %t.rev.without.bc -o - | FileCheck %s --implicit-check-not="{{foo|bar|baz}}"

; RUN: llvm-spirv %t.bc -spirv-text --spirv-preserve-auxdata --spirv-ext=+SPV_KHR_non_semantic_info,+SPV_INTEL_global_variable_decorations -o - | FileCheck %s --check-prefixes=CHECK-SPIRV,CHECK-SPIRV-NOEXT
; RUN: llvm-spirv %t.bc -o %t.spv --spirv-preserve-auxdata --spirv-ext=+SPV_INTEL_global_variable_decorations
; RUN: llvm-spirv %t.spv -to-text -o - | FileCheck %s --check-prefixes=CHECK-SPIRV,CHECK-SPIRV-NOEXT
; RUN: llvm-spirv -r --spirv-preserve-auxdata %t.spv -o %t.rev.bc
; RUN: llvm-dis %t.rev.bc -o - | FileCheck %s --check-prefix=CHECK-LLVM
; RUN: llvm-spirv -r %t.spv -o %t.rev.without.bc
; RUN: llvm-dis %t.rev.without.bc -o - | FileCheck %s --implicit-check-not="{{foo|bar|baz}}"

; Check SPIR-V versions in a format magic number + version
; CHECK-SPIRV-EXT: 119734787 65536
; CHECK-SPIRV-EXT: Extension "SPV_KHR_non_semantic_info"
; CHECK-SPIRV-NOEXT: 119734787 67072

; CHECK-SPIRV: ExtInstImport [[#Import:]] "NonSemantic.AuxData"

; CHECK-SPIRV: String [[#Attr0LHS:]] "sycl-device-global-size"
; CHECK-SPIRV: String [[#Attr0RHS:]] "32"
; CHECK-SPIRV: String [[#Attr1:]] "sycl-device-image-scope"
; CHECK-SPIRV: String [[#Attr2LHS:]] "sycl-host-access"
; CHECK-SPIRV: String [[#Attr2RHS:]] "0"
; CHECK-SPIRV: String [[#Attr3LHS:]] "sycl-unique-id"
; CHECK-SPIRV: String [[#Attr3RHS:]] "_Z20__AsanKernelMetadata"

; CHECK-SPIRV: Name [[#GVName:]] "__AsanKernelMetadata"

; CHECK-SPIRV: TypeVoid [[#VoidT:]]

; CHECK-SPIRV: ExtInst [[#VoidT]] [[#Attr0Inst:]] [[#Import]] NonSemanticAuxDataGlobalVariableAttribute [[#GVName]] [[#Attr0LHS]] [[#Attr0RHS]] {{$}}
; CHECK-SPIRV: ExtInst [[#VoidT]] [[#Attr1Inst:]] [[#Import]] NonSemanticAuxDataGlobalVariableAttribute [[#GVName]] [[#Attr1]] {{$}}
; CHECK-SPIRV: ExtInst [[#VoidT]] [[#Attr1Inst:]] [[#Import]] NonSemanticAuxDataGlobalVariableAttribute [[#GVName]] [[#Attr2LHS]] [[#Attr2RHS]] {{$}}
; CHECK-SPIRV: ExtInst [[#VoidT]] [[#Attr1Inst:]] [[#Import]] NonSemanticAuxDataGlobalVariableAttribute [[#GVName]] [[#Attr3LHS]] [[#Attr3RHS]] {{$}}

target triple = "spir64-unknown-unknown"

; CHECK-LLVM: @__AsanKernelMetadata = addrspace(1) global [1 x %structtype] [%structtype { i64 0, i64 92 }] #[[#GVIRAttr:]]
%structtype = type { i64, i64 }

@__AsanKernelMetadata = addrspace(1) global [1 x %structtype] [%structtype { i64 ptrtoint (ptr addrspace(2) null to i64), i64 92 }], !spirv.Decorations !0 #0

; CHECK-LLVM: attributes #[[#GVIRAttr]] = { "sycl-device-global-size"="32" "sycl-device-image-scope" "sycl-host-access"="0" "sycl-unique-id"="_Z20__AsanKernelMetadata" }
attributes #0 = { "sycl-device-global-size"="32" "sycl-device-image-scope" "sycl-host-access"="0" "sycl-unique-id"="_Z20__AsanKernelMetadata" }

!0 = !{!1}
!1 = !{i32 6147, i32 0, !"_Z20__AsanKernelMetadata"}

; CHECK-SPIRV-EXT-DISABLED: RequiresExtension: Feature requires the following SPIR-V extension:
; CHECK-SPIRV-EXT-DISABLED-NEXT: SPV_KHR_non_semantic_info
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
; RUN: llvm-as < %s -o %t.bc
; RUN: llvm-spirv %t.bc -o %t.spv --spirv-preserve-auxdata --spirv-max-version=1.5
; RUN: llvm-spirv %t.spv -to-text -o - | FileCheck %s --check-prefixes=CHECK-SPIRV,CHECK-SPIRV-EXT
; RUN: llvm-spirv -r --spirv-preserve-auxdata %t.spv -o %t.rev.bc
; RUN: llvm-dis %t.rev.bc -o - | FileCheck %s --check-prefix=CHECK-LLVM
; RUN: llvm-spirv -r %t.spv -o %t.rev.without.bc
; RUN: llvm-dis %t.rev.without.bc -o - | FileCheck %s --implicit-check-not="{{foo|bar|baz}}"

; RUN: llvm-spirv %t.bc -o %t.spv --spirv-preserve-auxdata
; RUN: llvm-spirv %t.spv -to-text -o - | FileCheck %s --check-prefixes=CHECK-SPIRV,CHECK-SPIRV-NOEXT
; RUN: llvm-spirv -r --spirv-preserve-auxdata %t.spv -o %t.rev.bc
; RUN: llvm-dis %t.rev.bc -o - | FileCheck %s --check-prefix=CHECK-LLVM
; RUN: llvm-spirv -r %t.spv -o %t.rev.without.bc
; RUN: llvm-dis %t.rev.without.bc -o - | FileCheck %s --implicit-check-not="{{foo|bar|baz}}"

; Check SPIR-V versions in a format magic number + version
; CHECK-SPIRV-EXT: 119734787 65536
; CHECK-SPIRV-EXT: Extension "SPV_KHR_non_semantic_info"
; CHECK-SPIRV-NOEXT: 119734787 67072

; CHECK-SPIRV: ExtInstImport [[#Import:]] "NonSemantic.AuxData"

; CHECK-SPIRV: String [[#MDName:]] "absolute_symbol"

; CHECK-SPIRV: Name [[#GVName:]] "a"

; CHECK-SPIRV: TypeInt [[#Int32T:]] 64 0
; CHECK-SPIRV: Constant [[#Int32T]] [[#MDValue0:]] 0
; CHECK-SPIRV: Constant [[#Int32T]] [[#MDValue1:]] 16

; CHECK-SPIRV: TypeVoid [[#VoidT:]]

; CHECK-SPIRV: ExtInst [[#VoidT]] [[#ValInst:]] [[#Import]] NonSemanticAuxDataGlobalVariableMetadata [[#GVName]] [[#MDName]] [[#MDValue0]] [[#MDValue1]] {{$}}

target triple = "spir64-unknown-unknown"

; CHECK-LLVM: @a = external addrspace(1) global i8, !absolute_symbol ![[#LLVMVal:]]
@a = external addrspace(1) global i8, !absolute_symbol !0

; CHECK-LLVM: ![[#LLVMVal]] = !{i64 0, i64 16}
!0 = !{i64 0, i64 16}
Loading