[llvm] [IR] Do not set `none` for function uwtable (PR #93387)

[Joshua Cao via llvm-commits]

https://github.com/caojoshua updated https://github.com/llvm/llvm-project/pull/93387

>From 78be05f968797490558ece3f70669b494ea5fb0b Mon Sep 17 00:00:00 2001
From: Joshua Cao <cao.joshua at yahoo.com>
Date: Sat, 25 May 2024 16:11:55 -0700
Subject: [PATCH 1/3] [IR] Do not set `none` for function uwtable

This avoids the pitfall where we set the uwtable to none:
```
func.setUWTableKind(llvm::UWTableKind::None)
```
`Attribute::getAsString()` would see an unknown attribute and fail an
assertion. In this patch, we assert that we do not see a None uwtable
kind.

This also skips the check of `UWTableKind::Async`. It is dominated by
the check of `UWTableKind::Default`, which has the same enum value
(nfc).
---
 llvm/include/llvm/IR/Function.h      |  3 ++-
 llvm/lib/CodeGen/MachineOutliner.cpp |  3 +--
 llvm/lib/IR/Attributes.cpp           | 10 +++-------
 3 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/llvm/include/llvm/IR/Function.h b/llvm/include/llvm/IR/Function.h
index cb514cde95b51..be745a0083cb7 100644
--- a/llvm/include/llvm/IR/Function.h
+++ b/llvm/include/llvm/IR/Function.h
@@ -654,7 +654,8 @@ class LLVM_EXTERNAL_VISIBILITY Function : public GlobalObject,
     return getUWTableKind() != UWTableKind::None;
   }
   void setUWTableKind(UWTableKind K) {
-    addFnAttr(Attribute::getWithUWTableKind(getContext(), K));
+    if (K != UWTableKind::None)
+      addFnAttr(Attribute::getWithUWTableKind(getContext(), K));
   }
   /// True if this function needs an unwind table.
   bool needsUnwindTableEntry() const {
diff --git a/llvm/lib/CodeGen/MachineOutliner.cpp b/llvm/lib/CodeGen/MachineOutliner.cpp
index dc2f5ef15206e..f174dd857def0 100644
--- a/llvm/lib/CodeGen/MachineOutliner.cpp
+++ b/llvm/lib/CodeGen/MachineOutliner.cpp
@@ -717,8 +717,7 @@ MachineFunction *MachineOutliner::createOutlinedFunction(
       [](UWTableKind K, const outliner::Candidate &C) {
         return std::max(K, C.getMF()->getFunction().getUWTableKind());
       });
-  if (UW != UWTableKind::None)
-    F->setUWTableKind(UW);
+  F->setUWTableKind(UW);
 
   BasicBlock *EntryBB = BasicBlock::Create(C, "entry", F);
   IRBuilder<> Builder(EntryBB);
diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp
index c8d6bdd423878..cc3a2ceb6de84 100644
--- a/llvm/lib/IR/Attributes.cpp
+++ b/llvm/lib/IR/Attributes.cpp
@@ -526,13 +526,9 @@ std::string Attribute::getAsString(bool InAttrGrp) const {
 
   if (hasAttribute(Attribute::UWTable)) {
     UWTableKind Kind = getUWTableKind();
-    if (Kind != UWTableKind::None) {
-      return Kind == UWTableKind::Default
-                 ? "uwtable"
-                 : ("uwtable(" +
-                    Twine(Kind == UWTableKind::Sync ? "sync" : "async") + ")")
-                       .str();
-    }
+    assert(Kind != UWTableKind::None &&
+           "uwtable attribute should not be none");
+    return Kind == UWTableKind::Default ? "uwtable" : "uwtable(sync)";
   }
 
   if (hasAttribute(Attribute::AllocKind)) {

>From 6f32102b92aa00ab04f3fa1d399da61faee47088 Mon Sep 17 00:00:00 2001
From: Joshua Cao <cao.joshua at yahoo.com>
Date: Sat, 25 May 2024 16:45:12 -0700
Subject: [PATCH 2/3] Fix formatting

---
 llvm/lib/IR/Attributes.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp
index cc3a2ceb6de84..7a3c9a99ee4d8 100644
--- a/llvm/lib/IR/Attributes.cpp
+++ b/llvm/lib/IR/Attributes.cpp
@@ -526,8 +526,7 @@ std::string Attribute::getAsString(bool InAttrGrp) const {
 
   if (hasAttribute(Attribute::UWTable)) {
     UWTableKind Kind = getUWTableKind();
-    assert(Kind != UWTableKind::None &&
-           "uwtable attribute should not be none");
+    assert(Kind != UWTableKind::None && "uwtable attribute should not be none");
     return Kind == UWTableKind::Default ? "uwtable" : "uwtable(sync)";
   }
 

>From e8147d9036a51133c637dd4f9a543b2cffdbd108 Mon Sep 17 00:00:00 2001
From: Joshua Cao <cao.joshua at yahoo.com>
Date: Sat, 1 Jun 2024 00:02:18 -0700
Subject: [PATCH 3/3] Remove uwtable attr setting UWTable::None. Add a unit
 test.

---
 llvm/include/llvm/IR/Function.h    |  4 +++-
 llvm/unittests/IR/FunctionTest.cpp | 23 +++++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/llvm/include/llvm/IR/Function.h b/llvm/include/llvm/IR/Function.h
index be745a0083cb7..5468cedb0815a 100644
--- a/llvm/include/llvm/IR/Function.h
+++ b/llvm/include/llvm/IR/Function.h
@@ -654,7 +654,9 @@ class LLVM_EXTERNAL_VISIBILITY Function : public GlobalObject,
     return getUWTableKind() != UWTableKind::None;
   }
   void setUWTableKind(UWTableKind K) {
-    if (K != UWTableKind::None)
+    if (K == UWTableKind::None)
+      removeFnAttr(Attribute::UWTable);
+    else
       addFnAttr(Attribute::getWithUWTableKind(getContext(), K));
   }
   /// True if this function needs an unwind table.
diff --git a/llvm/unittests/IR/FunctionTest.cpp b/llvm/unittests/IR/FunctionTest.cpp
index 8e77dfbb9dbd9..9aaff3ea33830 100644
--- a/llvm/unittests/IR/FunctionTest.cpp
+++ b/llvm/unittests/IR/FunctionTest.cpp
@@ -486,4 +486,27 @@ TEST(FunctionTest, EraseBBs) {
   It = F->erase(F->begin(), F->end());
   EXPECT_EQ(F->size(), 0u);
 }
+
+TEST(FunctionTest, UWTable) {
+  LLVMContext Ctx;
+  std::unique_ptr<Module> M = parseIR(Ctx, R"(
+    define void @foo() {
+     bb1:
+       ret void
+    }
+)");
+
+  Function &F = *M->getFunction("foo");
+
+  EXPECT_FALSE(F.hasUWTable());
+  EXPECT_TRUE(F.getUWTableKind() == UWTableKind::None);
+
+  F.setUWTableKind(UWTableKind::Async);
+  EXPECT_TRUE(F.hasUWTable());
+  EXPECT_TRUE(F.getUWTableKind() == UWTableKind::Async);
+
+  F.setUWTableKind(UWTableKind::None);
+  EXPECT_FALSE(F.hasUWTable());
+  EXPECT_TRUE(F.getUWTableKind() == UWTableKind::None);
+}
 } // end namespace