[clang] [llvm] [mlir] [IR] Replace alignment argument with attribute on masked intrinsics (PR #163802)

[Nikita Popov via cfe-commits]

================
@@ -725,6 +725,19 @@ Function *Intrinsic::getOrInsertDeclaration(Module *M, ID id,
   // There can never be multiple globals with the same name of different types,
   // because intrinsics must be a specific type.
   auto *FT = getType(M->getContext(), id, Tys);
+  Function *F = cast<Function>(
+      M->getOrInsertFunction(
+           Tys.empty() ? getName(id) : getName(id, Tys, M, FT), FT)
+          .getCallee());
+  if (F->getFunctionType() == FT)
+    return F;
+
+  // It's possible that a declaration for this intrinsic already exists with an
+  // incorrect signature, if the signature has changed, but this particular
+  // declaration has not been auto-upgraded yet. In that case, rename the
+  // invalid declaration and insert a new one with the correct signature. The
+  // invalid declaration will get upgraded later.
+  F->setName(F->getName() + ".invalid");
----------------
nikic wrote:

This happens e.g. in some X86 intrinsic upgrade tests. The flow is that in places like https://github.com/llvm/llvm-project/blob/64c511f6a88f607798db29a1af59aab4dd13ff20/llvm/lib/IR/AutoUpgrade.cpp#L2114 we are upgrading X86 intrinsics by creating llvm.masked.store. However, the same test may also include direct calls to llvm.masked.store. In this case, at the time the X86 intrinsic is upgraded, the llvm.masked.store may still have the old signature, which previously resulted in an assertion failure. The `.old` rename doesn't help because it would only happen later, when we get to that declaration.

(I have a prototype lying around somewhere that does not use named functions for intrinsics, and represents them via ID and signature only. That would avoid this kind of problem by design.)

https://github.com/llvm/llvm-project/pull/163802