[llvm][NFC] Adjust AutoUpdater 'masked' address-space access

[urnathan]

Summary

In working on memcpy et al, I noticed that many of the AutoUpdater's pattern matchers suffer the same issues of
(a) not commonizing similar handling of the intrinsic creation, and
(b) repeatedly checking for the same prefix, and
(c) if the name matched, but other conditions failed, it would fall through to check a different prefix (which could not match).

This one handles the 'masked.$FOO' intrinsics to avoid those deficiences.

Part 1 in a series of N.

[jroelofs]

LGTM

[arsenm]

llvm/lib/IR/AutoUpgrade.cpp

1056–1083

You can use consume_front for this instead of startswith+substr

[nikic]

Is this code needed at all? This looks like something that should be covered by intrinsic remangling. Explicit auto-upgrade should only be needed if you change the name or arguments, but not if you change only mangling.

[urnathan]

In D157763#4581908, @nikic wrote:

Is this code needed at all? This looks like something that should be covered by intrinsic remangling. Explicit auto-upgrade should only be needed if you change the name or arguments, but not if you change only mangling.

I don't know, and I'd rather not get further distracted -- think of this as an incomplete reduction towards zero source maybe?

llvm/lib/IR/AutoUpgrade.cpp

if (Name.startswith("invariant.group.barrier")) {
           Intrinsic::launder_invariant_group, ObjectPtr);
       return true;
 
     }
 
     break;
   }
   case 'm': {
-    if (Name.startswith("masked.load.")) {
-      Type *Tys[] = { F->getReturnType(), F->arg_begin()->getType() };
-      if (F->getName() !=
-          Intrinsic::getName(Intrinsic::masked_load, Tys, F->getParent())) {
-        rename(F);
-        NewFn = Intrinsic::getDeclaration(F->getParent(),
-                                          Intrinsic::masked_load,
-                                          Tys);
-        return true;
-      }
-    }
-    if (Name.startswith("masked.store.")) {
-      auto Args = F->getFunctionType()->params();
-      Type *Tys[] = { Args[0], Args[1] };
-      if (F->getName() !=
-          Intrinsic::getName(Intrinsic::masked_store, Tys, F->getParent())) {
-        rename(F);
-        NewFn = Intrinsic::getDeclaration(F->getParent(),
-                                          Intrinsic::masked_store,
-                                          Tys);
-        return true;
-      }
-    }
-    // Renaming gather/scatter intrinsics with no address space overloading
-    // to the new overload which includes an address space
-    if (Name.startswith("masked.gather.")) {
-      Type *Tys[] = {F->getReturnType(), F->arg_begin()->getType()};
-      if (F->getName() !=
-          Intrinsic::getName(Intrinsic::masked_gather, Tys, F->getParent())) {
+    StringRef MaskPfx = "masked.";
+    if (Name.startswith(MaskPfx)) {
+      // Renaming masked intrinsics with no address space overloading
+      // to the new overload, which includes an address space.
+      Intrinsic::ID ID =
+          StringSwitch<Intrinsic::ID>(Name.substr(MaskPfx.size()))
+              .StartsWith("load.", Intrinsic::masked_load)
+              .StartsWith("store.", Intrinsic::masked_store)
+              .StartsWith("gather.", Intrinsic::masked_gather)
+              .StartsWith("scatter.", Intrinsic::masked_scatter)
+              .Default(Intrinsic::not_intrinsic);
+      if (ID != Intrinsic::not_intrinsic) {
+        const auto *FT = F->getFunctionType();
+        SmallVector<Type *, 2> Tys;
+        if (ID == Intrinsic::masked_load || ID == Intrinsic::masked_gather)
+          // Loading operations overload on the return type.
+          Tys.push_back(FT->getReturnType());
+        Tys.push_back(FT->getParamType(0));
+        if (ID == Intrinsic::masked_store || ID == Intrinsic::masked_scatter)
+          // Store operations overload on the stored type.
+          Tys.push_back(FT->getParamType(1));
+        if (F->getName() != Intrinsic::getName(ID, Tys, F->getParent())) {
           rename(F);
-        NewFn = Intrinsic::getDeclaration(F->getParent(),
-                                          Intrinsic::masked_gather, Tys);
+          NewFn = Intrinsic::getDeclaration(F->getParent(), ID, Tys);
           return true;
         }
       }
-    if (Name.startswith("masked.scatter.")) {
-      auto Args = F->getFunctionType()->params();
-      Type *Tys[] = {Args[0], Args[1]};
-      if (F->getName() !=
-          Intrinsic::getName(Intrinsic::masked_scatter, Tys, F->getParent())) {
-        rename(F);
-        NewFn = Intrinsic::getDeclaration(F->getParent(),
-                                          Intrinsic::masked_scatter, Tys);
-        return true;
-      }
+      break; // No other 'masked.*'
     }
     // Updating the memory intrinsics (memcpy/memmove/memset) that have an
     // alignment parameter to embedding the alignment as an attribute of
     // the pointer args.
     if (unsigned ID = StringSwitch<unsigned>(Name)
                           .StartsWith("memcpy.", Intrinsic::memcpy)
                           .StartsWith("memmove.", Intrinsic::memmove)
                           .Default(0)) {