[DirectX] fix illegal behavior flag in module flags.

[python3kgae]

For DXIL which is based on llvm 3.7, max supported behavior flag for module flags is 6.

The commit will check all module flags, for behavior flag > 6, change it to 2 (Warning).

This is to fix the behavior flag part for #96912.

[llvmbot]

@llvm/pr-subscribers-backend-directx

Author: Xiang Li (python3kgae)

Changes

Remove all module flags other than "Dwarf Version" and "Debug Info Version".

Fixes #90773

---

Full diff: https://github.com/llvm/llvm-project/pull/96577.diff

3 Files Affected:

• (modified) llvm/lib/Target/DirectX/DXILPrepare.cpp (+26)
• (added) llvm/test/CodeGen/DirectX/clean-module-flags.ll (+23)
• (added) llvm/test/CodeGen/DirectX/clean-module-flags2.ll (+18)

diff --git a/llvm/lib/Target/DirectX/DXILPrepare.cpp b/llvm/lib/Target/DirectX/DXILPrepare.cpp
index 24be644d9fc0e..500989a4c2016 100644
--- a/llvm/lib/Target/DirectX/DXILPrepare.cpp
+++ b/llvm/lib/Target/DirectX/DXILPrepare.cpp
@@ -116,6 +116,30 @@ static void removeStringFunctionAttributes(Function &F,
   F.removeRetAttrs(DeadAttrs);
 }
 
+static void cleanModuleFlags(Module &M) {
+  NamedMDNode *MDFlags = M.getModuleFlagsMetadata();
+  if (!MDFlags)
+    return;
+
+  StringSet<> LiveKeys = {"Dwarf Version", "Debug Info Version"};
+
+  SmallVector<llvm::Module::ModuleFlagEntry> FlagEntries;
+  M.getModuleFlagsMetadata(FlagEntries);
+  SmallVector<llvm::Module::ModuleFlagEntry> LiveFlagEntries;
+  for (auto &Flag : FlagEntries) {
+    if (LiveKeys.count(Flag.Key->getString()))
+      LiveFlagEntries.push_back(Flag);
+  }
+
+  if (LiveFlagEntries.size() == FlagEntries.size())
+    return;
+
+  MDFlags->eraseFromParent();
+
+  for (auto &Flag : LiveFlagEntries)
+    M.addModuleFlag(Flag.Behavior, Flag.Key->getString(), Flag.Val);
+}
+
 class DXILPrepareModule : public ModulePass {
 
   static Value *maybeGenerateBitcast(IRBuilder<> &Builder,
@@ -213,6 +237,8 @@ class DXILPrepareModule : public ModulePass {
         }
       }
     }
+    // Remove flags not for DXIL.
+    cleanModuleFlags(M);
     return true;
   }
 
diff --git a/llvm/test/CodeGen/DirectX/clean-module-flags.ll b/llvm/test/CodeGen/DirectX/clean-module-flags.ll
new file mode 100644
index 0000000000000..0104cc5fa1b4d
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/clean-module-flags.ll
@@ -0,0 +1,23 @@
+; RUN: opt -S -dxil-prepare < %s | FileCheck %s
+
+; Make sure non-dxil module flags are removed.
+; CHECK-NOT:"wchar_size"
+; CHECK-NOT:"frame-pointer"
+
+; CHECK:!llvm.module.flags = !{!0, !1}
+; CHECK:!0 = !{i32 7, !"Dwarf Version", i32 2}
+; CHECK:!1 = !{i32 2, !"Debug Info Version", i32 3}
+
+; Function Attrs: nounwind memory(none)
+define void @main() local_unnamed_addr #0 {
+entry:
+  ret void
+}
+
+attributes #0 = { nounwind memory(none) }
+!llvm.module.flags = !{!0, !1, !2, !3}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{i32 7, !"frame-pointer", i32 2}
+!2 = !{i32 7, !"Dwarf Version", i32 2}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
diff --git a/llvm/test/CodeGen/DirectX/clean-module-flags2.ll b/llvm/test/CodeGen/DirectX/clean-module-flags2.ll
new file mode 100644
index 0000000000000..068a908b2b406
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/clean-module-flags2.ll
@@ -0,0 +1,18 @@
+; RUN: opt -S -dxil-prepare < %s | FileCheck %s
+
+; CHECK: define void @main()
+; Make sure non-dxil module flags are removed.
+; CHECK-NOT:"wchar_size"
+; CHECK-NOT:"frame-pointer"
+
+; Function Attrs: nounwind memory(none)
+define void @main() local_unnamed_addr #0 {
+entry:
+  ret void
+}
+
+attributes #0 = { nounwind memory(none) }
+!llvm.module.flags = !{!0, !1}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{i32 7, !"frame-pointer", i32 2}

[damyanp]

It looks like clean-module-flags.ll and clean-module-flags2.ll are testing the same thing? Or am I missing something?

[python3kgae]

clean-module-flags.ll has Dwarf Version and Debug Info Version.

[damyanp]

Seems overkill to me - and by this precedent we'd need to add new combinations of flags is new ones ever show up. But probably doesn't hurt.

[llvm-beanz]

Can we give this new test a more descriptive name? This is actually testing updating the flag behavior field, so how about "update-module-flag-behavior.ll"?

[python3kgae]

Updated to legalize module flag.

[damyanp]

Interested in why:

• no CHECK line for wchar_size?
• CHECK-NOT is used for frame-pointer, but CHECK is used for Dwarf Version

[python3kgae]

For DXIL, it is legal to not have wchar_size and frame-pointer.
But remove Dwarf Version will lost information for DXIL.

[llvm-beanz]

This check doesn't seem correct. The code change here doesn't remove the flag it changes the behavior value. We should check for the updated behavior not check that the old behavior doesn't exist.

[python3kgae]

Updated.

[bob80905]

Could you explain why the flag behavior is set to warning? If I understand, you have 6 options to choose from, so why pick that one?

[bob80905]

Could you explain why the flag behavior is set to warning? If I understand, you have 6 options to choose from, so why pick that one?

I read the issue, I think I get it now.