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] Adjust Autoupdater's llvm prefix detection #74142

Merged
merged 1 commit into from
Dec 2, 2023

Conversation

urnathan
Copy link
Contributor

@urnathan urnathan commented Dec 1, 2023

Use consume_front to swallow the 'llvm.' prefix, and 'empty' to check there's at least one character left. It does not seem worth while checking for at least 2 characters left -- any well formed IR will have several characters remaining anyway.

probably the simplest of my autoupdater changes.

Use consume_front to swallow the 'llvm.' prefix, and 'empty' to check
there's at least one character left.  It does not seem worth while
checking for at least 2 characters left -- any well formed IR will
have several characters remaining anyway.
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

@urnathan urnathan marked this pull request as ready for review December 2, 2023 16:57
@llvmbot llvmbot added the llvm:ir label Dec 2, 2023
@urnathan urnathan merged commit d04a4a0 into llvm:main Dec 2, 2023
5 checks passed
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 2, 2023

@llvm/pr-subscribers-llvm-ir

Author: Nathan Sidwell (urnathan)

Changes

Use consume_front to swallow the 'llvm.' prefix, and 'empty' to check there's at least one character left. It does not seem worth while checking for at least 2 characters left -- any well formed IR will have several characters remaining anyway.

probably the simplest of my autoupdater changes.


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

1 Files Affected:

  • (modified) llvm/lib/IR/AutoUpgrade.cpp (+3-3)
diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
index 9d546d3a5e9b1..7a40f24661caf 100644
--- a/llvm/lib/IR/AutoUpgrade.cpp
+++ b/llvm/lib/IR/AutoUpgrade.cpp
@@ -704,11 +704,11 @@ static Intrinsic::ID ShouldUpgradeNVPTXBF16Intrinsic(StringRef Name) {
 static bool UpgradeIntrinsicFunction1(Function *F, Function *&NewFn) {
   assert(F && "Illegal to upgrade a non-existent Function.");
 
-  // Quickly eliminate it, if it's not a candidate.
   StringRef Name = F->getName();
-  if (Name.size() <= 7 || !Name.starts_with("llvm."))
+
+  // Quickly eliminate it, if it's not a candidate.
+  if (!Name.consume_front("llvm.") || Name.empty())
     return false;
-  Name = Name.substr(5); // Strip off "llvm."
 
   switch (Name[0]) {
   default: break;

@urnathan urnathan deleted the push/autoupdater-misc branch February 2, 2024 19:32
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.

None yet

3 participants