[llvm][NFC] Autoupdater AMD intrinsic detection

[urnathan]

Here's a refactor of the amd autoupdater, using prefix stripping and commonizing the replacement fndecl creation.

[llvmbot]

@llvm/pr-subscribers-llvm-ir

Author: Nathan Sidwell (urnathan)

Changes

Here's a refactor of the amd autoupdater, using prefix stripping and commonizing the replacement fndecl creation.

---

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

1 Files Affected:

• (modified) llvm/lib/IR/AutoUpgrade.cpp (+19-18)

diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
index 63c4b2c71299900..4a9b8e10b43e3f3 100644
--- a/llvm/lib/IR/AutoUpgrade.cpp
+++ b/llvm/lib/IR/AutoUpgrade.cpp
@@ -945,29 +945,30 @@ static bool UpgradeIntrinsicFunction1(Function *F, Function *&NewFn) {
       return true;
 
     if (Name.consume_front("amdgcn.")) {
-      if (Name == "alignbit") {
-        // Target specific intrinsic became redundant
-        NewFn = Intrinsic::getDeclaration(F->getParent(), Intrinsic::fshr,
-                                          {F->getReturnType()});
-        return true;
-      }
 
-      if (Name.starts_with("atomic.inc") || Name.starts_with("atomic.dec")) {
-        // This was replaced with atomicrmw uinc_wrap and udec_wrap, so there's no
-        // new declaration.
-        NewFn = nullptr;
+      Intrinsic::ID ID = StringSwitch<Intrinsic::ID>(Name)
+                             .Case("alignbit", Intrinsic::fshr)
+                             .StartsWith("ldexp.", Intrinsic::ldexp)
+                             .Default(Intrinsic::not_intrinsic);
+      if (ID != Intrinsic::not_intrinsic) {
+        // Some target-specific intrinsics became redundant.
+        SmallVector<Type *, 2> Tys;
+        Tys.push_back(F->getReturnType());
+        if (ID == Intrinsic::ldexp)
+          Tys.push_back(F->getArg(1)->getType());
+        NewFn = Intrinsic::getDeclaration(F->getParent(), ID, Tys);
         return true;
       }
 
-      if (Name.starts_with("ldexp.")) {
-        // Target specific intrinsic became redundant
-        NewFn = Intrinsic::getDeclaration(
-          F->getParent(), Intrinsic::ldexp,
-          {F->getReturnType(), F->getArg(1)->getType()});
-        return true;
-      }
+      if (Name.consume_front("atomic."))
+        if (Name.starts_with("inc") || Name.starts_with("dec")) {
+          // These were replaced with atomicrmw uinc_wrap and udec_wrap, so
+          // there's no new declaration.
+          NewFn = nullptr;
+          return true;
+        }
+      break; // No other 'amdgcn.*'
     }
-
     break;
   }
   case 'c': {

[nikic]

I feel like this one isn't an improvement. I don't think the alignbit and ldexp cases should be conflated. They don't really have any relation and different signatures.

[urnathan]

You're right, I'd got too fixated on StringSwitch. Here's an update just addressing the 'atomic' ops, where the prefix check can help.

[nikic]

I don't mind the change, but I personally probably wouldn't do it -- don't think that handling a common prefix for just two intrinsic really adds clarity. I'll leave it up to you.