-
Notifications
You must be signed in to change notification settings - Fork 14.8k
[DXIL] Remove incompatible metadata types when preparing DXIL. #136386
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
Changes from 6 commits
c1a89d6
0399c6f
19de155
416084c
84ad1d4
fb2932e
b18f093
58683b6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -161,6 +161,15 @@ class DXILPrepareModule : public ModulePass { | |
Builder.getPtrTy(PtrTy->getAddressSpace()))); | ||
} | ||
|
||
static std::array<unsigned, 6> getCompatibleInstructionMDs(llvm::Module &M) { | ||
return {M.getMDKindID("dx.nonuniform"), | ||
M.getMDKindID("dx.controlflow.hints"), | ||
M.getMDKindID("dx.precise"), | ||
llvm::LLVMContext::MD_range, | ||
llvm::LLVMContext::MD_alias_scope, | ||
llvm::LLVMContext::MD_noalias}; | ||
} | ||
|
||
public: | ||
bool runOnModule(Module &M) override { | ||
PointerTypeMap PointerTypes = PointerTypeAnalysis::run(M); | ||
|
@@ -176,6 +185,9 @@ class DXILPrepareModule : public ModulePass { | |
VersionTuple ValVer = MetadataInfo.ValidatorVersion; | ||
bool SkipValidation = ValVer.getMajor() == 0 && ValVer.getMinor() == 0; | ||
|
||
// construct whitelist of valid metadata node kinds | ||
std::array<unsigned, 6> DXILCompatibleMDs = getCompatibleInstructionMDs(M); | ||
|
||
for (auto &F : M.functions()) { | ||
F.removeFnAttrs(AttrMask); | ||
F.removeRetAttrs(AttrMask); | ||
|
@@ -189,6 +201,9 @@ class DXILPrepareModule : public ModulePass { | |
for (auto &BB : F) { | ||
IRBuilder<> Builder(&BB); | ||
for (auto &I : make_early_inc_range(BB)) { | ||
|
||
I.dropUnknownNonDebugMetadata(DXILCompatibleMDs); | ||
|
||
if (I.getOpcode() == Instruction::FNeg) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DXILPrepare has a bunch of overlap with the legalization pass. Seems like MetaData might not be able to move because legalization is a function level pass and this is a module level one, but at a minimum it seems like FNeg -> FSub should move. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. I think DXILPrepare should do things like translate attributes and metadata, insert no-op bitcasts to appease the bitcode writer, and that sort of thing. Replacing ops with their equivalents should squarely be a part of legalization, not here. I've filed #137685 |
||
Builder.SetInsertPoint(&I); | ||
Value *In = I.getOperand(0); | ||
|
bogner marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
; RUN: opt -S --dxil-prepare %s | FileCheck %s | ||
|
||
; This test tests the whitelist inside of DxilPrepare.cpp. | ||
; It ensures that certain metadata nodes are removed that aren't | ||
; in the whitelist, and that certain nodes may remain that | ||
; are on the whitelist. | ||
bogner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
target triple = "dxilv1.0-unknown-shadermodel6.0-compute" | ||
|
||
; Function Attrs: noinline nounwind memory(readwrite, inaccessiblemem: none) | ||
define void @main(i32* %ptr) { | ||
entry: | ||
; metadata ID changes to 0 once the current !0 and !1 are removed | ||
; since they aren't in the whitelist. range needs a payload. | ||
bogner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
; CHECK: %val = load i32, ptr %ptr, align 4, !range !0 | ||
bogner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
%val = load i32, ptr %ptr, align 4, !range !2 | ||
|
||
; dx.nonuniform is a valid metadata node kind on the whitelist, | ||
; so give it a bogus payload and ensure it sticks around | ||
; CHECK-next: %cmp.i1.not = icmp eq i32 1, 0, !dx.nonuniform !0 | ||
%cmp.i1.not = icmp eq i32 1, 0, !dx.nonuniform !2 | ||
bogner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
br i1 %cmp.i1.not, label %_Z4mainDv3_j.exit, label %for.body.i | ||
|
||
for.body.i: ; preds = %entry | ||
%cmp.i = icmp ult i32 1, 2 | ||
br i1 %cmp.i, label %for.body.i, label %_Z4mainDv3_j.exit, !llvm.loop !0 | ||
|
||
_Z4mainDv3_j.exit: ; preds = %for.body.i, %entry | ||
ret void | ||
} | ||
|
||
; CHECK: !0 = !{i32 1, i32 5} | ||
; this next check line checks that nothing comes after the above check line. | ||
; No more metadata should be necessary, the rest (the current 0 and 1) | ||
; should be removed. | ||
; CHECK-NOT: !{!"llvm.loop.mustprogress"} | ||
!0 = distinct !{!0, !1} | ||
!1 = !{!"llvm.loop.mustprogress"} | ||
!2 = !{i32 1, i32 5} |
Uh oh!
There was an error while loading. Please reload this page.