Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[llvm][NFC] Autoupdater x86 intrinsic selection #73046

Merged
merged 2 commits into from
Nov 25, 2023

Conversation

urnathan
Copy link
Contributor

Another part of the x86 intrinsic autoupdater can benefit from (a) sorting the intrinsics, (b) doing some prefix checking, (c) using StringSwitch to find a replacement ID. This replaces 30 checks with 5 prefix checks and 26 Upgrade calls wth 9 calls. This also removes the uses of 'substr(MAGIC_CONSTANT)'.

Another part of the x86 intrinsic autoupdater can benefit from (a)
sorting the intrinsics, (b) doing some prefix checking, (c) using
StringSwitch to find a replacement ID. This replaces 30 checks with 5
prefix checks and 26 Upgrade calls wth 9 calls. This also removes the
uses of 'substr(MAGIC_CONSTANT)'.
@urnathan urnathan marked this pull request as ready for review November 21, 2023 23:29
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 21, 2023

@llvm/pr-subscribers-llvm-ir

Author: Nathan Sidwell (urnathan)

Changes

Another part of the x86 intrinsic autoupdater can benefit from (a) sorting the intrinsics, (b) doing some prefix checking, (c) using StringSwitch to find a replacement ID. This replaces 30 checks with 5 prefix checks and 26 Upgrade calls wth 9 calls. This also removes the uses of 'substr(MAGIC_CONSTANT)'.


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

1 Files Affected:

  • (modified) llvm/lib/IR/AutoUpgrade.cpp (+100-101)
diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
index 63c4b2c71299900..be23fa539d875c6 100644
--- a/llvm/lib/IR/AutoUpgrade.cpp
+++ b/llvm/lib/IR/AutoUpgrade.cpp
@@ -486,10 +486,8 @@ static bool ShouldUpgradeX86Intrinsic(Function *F, StringRef Name) {
 static bool UpgradeX86IntrinsicFunction(Function *F, StringRef Name,
                                         Function *&NewFn) {
   // Only handle intrinsics that start with "x86.".
-  if (!Name.starts_with("x86."))
+  if (!Name.consume_front("x86."))
     return false;
-  // Remove "x86." prefix.
-  Name = Name.substr(4);
 
   if (ShouldUpgradeX86Intrinsic(F, Name)) {
     NewFn = nullptr;
@@ -507,113 +505,114 @@ static bool UpgradeX86IntrinsicFunction(Function *F, StringRef Name,
     return true;
   }
 
+  Intrinsic::ID ID;
+
   // SSE4.1 ptest functions may have an old signature.
-  if (Name.starts_with("sse41.ptest")) { // Added in 3.2
-    if (Name.substr(11) == "c")
-      return UpgradePTESTIntrinsic(F, Intrinsic::x86_sse41_ptestc, NewFn);
-    if (Name.substr(11) == "z")
-      return UpgradePTESTIntrinsic(F, Intrinsic::x86_sse41_ptestz, NewFn);
-    if (Name.substr(11) == "nzc")
-      return UpgradePTESTIntrinsic(F, Intrinsic::x86_sse41_ptestnzc, NewFn);
+  if (Name.consume_front("sse41.ptest")) { // Added in 3.2
+    ID = StringSwitch<Intrinsic::ID>(Name)
+             .Case("c", Intrinsic::x86_sse41_ptestc)
+             .Case("z", Intrinsic::x86_sse41_ptestz)
+             .Case("nzc", Intrinsic::x86_sse41_ptestnzc)
+             .Default(Intrinsic::not_intrinsic);
+    if (ID != Intrinsic::not_intrinsic)
+      return UpgradePTESTIntrinsic(F, ID, NewFn);
+
+    return false;
   }
+
   // Several blend and other instructions with masks used the wrong number of
   // bits.
-  if (Name == "sse41.insertps") // Added in 3.6
-    return UpgradeX86IntrinsicsWith8BitMask(F, Intrinsic::x86_sse41_insertps,
-                                            NewFn);
-  if (Name == "sse41.dppd") // Added in 3.6
-    return UpgradeX86IntrinsicsWith8BitMask(F, Intrinsic::x86_sse41_dppd,
-                                            NewFn);
-  if (Name == "sse41.dpps") // Added in 3.6
-    return UpgradeX86IntrinsicsWith8BitMask(F, Intrinsic::x86_sse41_dpps,
-                                            NewFn);
-  if (Name == "sse41.mpsadbw") // Added in 3.6
-    return UpgradeX86IntrinsicsWith8BitMask(F, Intrinsic::x86_sse41_mpsadbw,
-                                            NewFn);
-  if (Name == "avx.dp.ps.256") // Added in 3.6
-    return UpgradeX86IntrinsicsWith8BitMask(F, Intrinsic::x86_avx_dp_ps_256,
-                                            NewFn);
-  if (Name == "avx2.mpsadbw") // Added in 3.6
-    return UpgradeX86IntrinsicsWith8BitMask(F, Intrinsic::x86_avx2_mpsadbw,
-                                            NewFn);
-  if (Name == "avx512.mask.cmp.pd.128") // Added in 7.0
-    return UpgradeX86MaskedFPCompare(F, Intrinsic::x86_avx512_mask_cmp_pd_128,
-                                     NewFn);
-  if (Name == "avx512.mask.cmp.pd.256") // Added in 7.0
-    return UpgradeX86MaskedFPCompare(F, Intrinsic::x86_avx512_mask_cmp_pd_256,
-                                     NewFn);
-  if (Name == "avx512.mask.cmp.pd.512") // Added in 7.0
-    return UpgradeX86MaskedFPCompare(F, Intrinsic::x86_avx512_mask_cmp_pd_512,
-                                     NewFn);
-  if (Name == "avx512.mask.cmp.ps.128") // Added in 7.0
-    return UpgradeX86MaskedFPCompare(F, Intrinsic::x86_avx512_mask_cmp_ps_128,
-                                     NewFn);
-  if (Name == "avx512.mask.cmp.ps.256") // Added in 7.0
-    return UpgradeX86MaskedFPCompare(F, Intrinsic::x86_avx512_mask_cmp_ps_256,
-                                     NewFn);
-  if (Name == "avx512.mask.cmp.ps.512") // Added in 7.0
-    return UpgradeX86MaskedFPCompare(F, Intrinsic::x86_avx512_mask_cmp_ps_512,
-                                     NewFn);
-  if (Name == "avx512bf16.cvtne2ps2bf16.128") // Added in 9.0
-    return UpgradeX86BF16Intrinsic(
-        F, Intrinsic::x86_avx512bf16_cvtne2ps2bf16_128, NewFn);
-  if (Name == "avx512bf16.cvtne2ps2bf16.256") // Added in 9.0
-    return UpgradeX86BF16Intrinsic(
-        F, Intrinsic::x86_avx512bf16_cvtne2ps2bf16_256, NewFn);
-  if (Name == "avx512bf16.cvtne2ps2bf16.512") // Added in 9.0
-    return UpgradeX86BF16Intrinsic(
-        F, Intrinsic::x86_avx512bf16_cvtne2ps2bf16_512, NewFn);
-  if (Name == "avx512bf16.mask.cvtneps2bf16.128") // Added in 9.0
-    return UpgradeX86BF16Intrinsic(
-        F, Intrinsic::x86_avx512bf16_mask_cvtneps2bf16_128, NewFn);
-  if (Name == "avx512bf16.cvtneps2bf16.256") // Added in 9.0
-    return UpgradeX86BF16Intrinsic(
-        F, Intrinsic::x86_avx512bf16_cvtneps2bf16_256, NewFn);
-  if (Name == "avx512bf16.cvtneps2bf16.512") // Added in 9.0
-    return UpgradeX86BF16Intrinsic(
-        F, Intrinsic::x86_avx512bf16_cvtneps2bf16_512, NewFn);
-  if (Name == "avx512bf16.dpbf16ps.128") // Added in 9.0
-    return UpgradeX86BF16DPIntrinsic(
-        F, Intrinsic::x86_avx512bf16_dpbf16ps_128, NewFn);
-  if (Name == "avx512bf16.dpbf16ps.256") // Added in 9.0
-    return UpgradeX86BF16DPIntrinsic(
-        F, Intrinsic::x86_avx512bf16_dpbf16ps_256, NewFn);
-  if (Name == "avx512bf16.dpbf16ps.512") // Added in 9.0
-    return UpgradeX86BF16DPIntrinsic(
-        F, Intrinsic::x86_avx512bf16_dpbf16ps_512, NewFn);
-
-  // frcz.ss/sd may need to have an argument dropped. Added in 3.2
-  if (Name.starts_with("xop.vfrcz.ss") && F->arg_size() == 2) {
-    rename(F);
-    NewFn = Intrinsic::getDeclaration(F->getParent(),
-                                      Intrinsic::x86_xop_vfrcz_ss);
-    return true;
+
+  // Added in 3.6
+  ID = StringSwitch<Intrinsic::ID>(Name)
+           .Case("sse41.insertps", Intrinsic::x86_sse41_insertps)
+           .Case("sse41.dppd", Intrinsic::x86_sse41_dppd)
+           .Case("sse41.dpps", Intrinsic::x86_sse41_dpps)
+           .Case("sse41.mpsadbw", Intrinsic::x86_sse41_mpsadbw)
+           .Case("avx.dp.ps.256", Intrinsic::x86_avx_dp_ps_256)
+           .Case("avx2.mpsadbw", Intrinsic::x86_avx2_mpsadbw)
+           .Default(Intrinsic::not_intrinsic);
+  if (ID != Intrinsic::not_intrinsic)
+    return UpgradeX86IntrinsicsWith8BitMask(F, ID, NewFn);
+
+  if (Name.consume_front("avx512.mask.cmp.")) {
+    // Added in 7.0
+    ID = StringSwitch<Intrinsic::ID>(Name)
+             .Case("pd.128", Intrinsic::x86_avx512_mask_cmp_pd_128)
+             .Case("pd.256", Intrinsic::x86_avx512_mask_cmp_pd_256)
+             .Case("pd.512", Intrinsic::x86_avx512_mask_cmp_pd_512)
+             .Case("ps.128", Intrinsic::x86_avx512_mask_cmp_ps_128)
+             .Case("ps.256", Intrinsic::x86_avx512_mask_cmp_ps_256)
+             .Case("ps.512", Intrinsic::x86_avx512_mask_cmp_ps_512)
+             .Default(Intrinsic::not_intrinsic);
+    if (ID != Intrinsic::not_intrinsic)
+      return UpgradeX86MaskedFPCompare(F, ID, NewFn);
+    return false; // No other 'x86.avx523.mask.cmp.*'.
   }
-  if (Name.starts_with("xop.vfrcz.sd") && F->arg_size() == 2) {
-    rename(F);
-    NewFn = Intrinsic::getDeclaration(F->getParent(),
-                                      Intrinsic::x86_xop_vfrcz_sd);
-    return true;
+
+  if (Name.consume_front("avx512bf16.")) {
+    // Added in 9.0
+    ID = StringSwitch<Intrinsic::ID>(Name)
+             .Case("cvtne2ps2bf16.128",
+                   Intrinsic::x86_avx512bf16_cvtne2ps2bf16_128)
+             .Case("cvtne2ps2bf16.256",
+                   Intrinsic::x86_avx512bf16_cvtne2ps2bf16_256)
+             .Case("cvtne2ps2bf16.512",
+                   Intrinsic::x86_avx512bf16_cvtne2ps2bf16_512)
+             .Case("mask.cvtneps2bf16.128",
+                   Intrinsic::x86_avx512bf16_mask_cvtneps2bf16_128)
+             .Case("cvtneps2bf16.256",
+                   Intrinsic::x86_avx512bf16_cvtneps2bf16_256)
+             .Case("cvtneps2bf16.512",
+                   Intrinsic::x86_avx512bf16_cvtneps2bf16_512)
+             .Default(Intrinsic::not_intrinsic);
+    if (ID != Intrinsic::not_intrinsic)
+      return UpgradeX86BF16Intrinsic(F, ID, NewFn);
+
+    // Added in 9.0
+    ID = StringSwitch<Intrinsic::ID>(Name)
+             .Case("dpbf16ps.128", Intrinsic::x86_avx512bf16_dpbf16ps_128)
+             .Case("dpbf16ps.256", Intrinsic::x86_avx512bf16_dpbf16ps_256)
+             .Case("dpbf16ps.512", Intrinsic::x86_avx512bf16_dpbf16ps_512)
+             .Default(Intrinsic::not_intrinsic);
+    if (ID != Intrinsic::not_intrinsic)
+      return UpgradeX86BF16DPIntrinsic(F, ID, NewFn);
+    return false; // No other 'x86.avx512bf16.*'.
   }
-  // Upgrade any XOP PERMIL2 index operand still using a float/double vector.
-  if (Name.starts_with("xop.vpermil2")) { // Added in 3.9
-    auto Idx = F->getFunctionType()->getParamType(2);
-    if (Idx->isFPOrFPVectorTy()) {
+
+  if (Name.consume_front("xop.")) {
+    Intrinsic::ID ID = Intrinsic::not_intrinsic;
+    if (Name.startswith("vpermil2")) { // Added in 3.9
+      // Upgrade any XOP PERMIL2 index operand still using a float/double
+      // vector.
+      auto Idx = F->getFunctionType()->getParamType(2);
+      if (Idx->isFPOrFPVectorTy()) {
+        unsigned IdxSize = Idx->getPrimitiveSizeInBits();
+        unsigned EltSize = Idx->getScalarSizeInBits();
+        if (EltSize == 64 && IdxSize == 128)
+          ID = Intrinsic::x86_xop_vpermil2pd;
+        else if (EltSize == 32 && IdxSize == 128)
+          ID = Intrinsic::x86_xop_vpermil2ps;
+        else if (EltSize == 64 && IdxSize == 256)
+          ID = Intrinsic::x86_xop_vpermil2pd_256;
+        else
+          ID = Intrinsic::x86_xop_vpermil2ps_256;
+      }
+    } else if (F->arg_size() == 2)
+      // frcz.ss/sd may need to have an argument dropped. Added in 3.2
+      ID = StringSwitch<Intrinsic::ID>(Name)
+               .Case("vfrcz.ss", Intrinsic::x86_xop_vfrcz_ss)
+               .Case("vfrcz.sd", Intrinsic::x86_xop_vfrcz_sd)
+               .Default(Intrinsic::not_intrinsic);
+    else
+      ID = Intrinsic::not_intrinsic;
+
+    if (ID != Intrinsic::not_intrinsic) {
       rename(F);
-      unsigned IdxSize = Idx->getPrimitiveSizeInBits();
-      unsigned EltSize = Idx->getScalarSizeInBits();
-      Intrinsic::ID Permil2ID;
-      if (EltSize == 64 && IdxSize == 128)
-        Permil2ID = Intrinsic::x86_xop_vpermil2pd;
-      else if (EltSize == 32 && IdxSize == 128)
-        Permil2ID = Intrinsic::x86_xop_vpermil2ps;
-      else if (EltSize == 64 && IdxSize == 256)
-        Permil2ID = Intrinsic::x86_xop_vpermil2pd_256;
-      else
-        Permil2ID = Intrinsic::x86_xop_vpermil2ps_256;
-      NewFn = Intrinsic::getDeclaration(F->getParent(), Permil2ID);
+      NewFn = Intrinsic::getDeclaration(F->getParent(), ID);
       return true;
     }
+    return false; // No other 'x86.xop.*'
   }
 
   if (Name == "seh.recoverfp") {

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

llvm/lib/IR/AutoUpgrade.cpp Outdated Show resolved Hide resolved
remove unneeded else.
@urnathan urnathan merged commit fcf5ac8 into llvm:main Nov 25, 2023
3 checks passed
@urnathan urnathan deleted the push/autoupdater-x86-2 branch December 1, 2023 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants