Fix error message when regalloc eviction advisor analysis could not be created

[hiraditya]

No description provided.

[llvmbot]

@llvm/pr-subscribers-mlgo

Author: AdityaK (hiraditya)

Changes

---

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

2 Files Affected:

• (modified) llvm/lib/CodeGen/RegAllocEvictionAdvisor.cpp (+1-1)
• (modified) llvm/test/CodeGen/MLRegAlloc/default-eviction-advisor.ll (+2-2)

diff --git a/llvm/lib/CodeGen/RegAllocEvictionAdvisor.cpp b/llvm/lib/CodeGen/RegAllocEvictionAdvisor.cpp
index 81f3d2c8099f18b..47ad9c168b92367 100644
--- a/llvm/lib/CodeGen/RegAllocEvictionAdvisor.cpp
+++ b/llvm/lib/CodeGen/RegAllocEvictionAdvisor.cpp
@@ -83,7 +83,7 @@ class DefaultEvictionAdvisorAnalysis final
   bool doInitialization(Module &M) override {
     if (NotAsRequested)
       M.getContext().emitError("Requested regalloc eviction advisor analysis "
-                               "could be created. Using default");
+                               "could not be created. Using default");
     return RegAllocEvictionAdvisorAnalysis::doInitialization(M);
   }
   const bool NotAsRequested;
diff --git a/llvm/test/CodeGen/MLRegAlloc/default-eviction-advisor.ll b/llvm/test/CodeGen/MLRegAlloc/default-eviction-advisor.ll
index 0f4485e2de1d926..337fbb767f36c96 100644
--- a/llvm/test/CodeGen/MLRegAlloc/default-eviction-advisor.ll
+++ b/llvm/test/CodeGen/MLRegAlloc/default-eviction-advisor.ll
@@ -16,5 +16,5 @@ define void @f2(i64 %lhs, i64 %rhs, i64* %addr) {
   ret void
 }
 
-; CHECK: Requested regalloc eviction advisor analysis could be created. Using default
-; DEFAULT-NOT: Requested regalloc eviction advisor analysis could be created. Using default
+; CHECK: Requested regalloc eviction advisor analysis could not be created. Using default
+; DEFAULT-NOT: Requested regalloc eviction advisor analysis could not be created. Using default

[mtrofin]

lgtm, thanks!

[pcc]

Maybe also remove the words "Using default"? It's an error, the compiler isn't going to "use" anything.

[boomanaiden154]

Maybe also remove the words "Using default"? It's an error, the compiler isn't going to "use" anything.

It's an error, but not an error that causes an exit, as the state is still fine to continue on from. The variable comes from https://github.com/hiraditya/llvm-project/blob/0175a1e4d33720ed7e827b3db5a36f88bdd790a3/llvm/lib/CodeGen/RegAllocEvictionAdvisor.cpp#L93. If the user requests an ML analysis and it can't be created (eg due to the user not building with the ML stuff enabled), NotAsRequested is set to true, and the default advisor is constructed. Everything still works with the default advisor, it's just not what the user requested.

[pcc]

I think you're referring to an atypical case where a program prints an error and continues anyway. By default I would expect programs to fail if they encounter an error, and it's confusing to have the error message assume otherwise. It's fine if the error message doesn't say exactly what happened if the user passed some "ignore all errors" flag.

[boomanaiden154]

Yes, it is an atypical case. It very much depends on the type of error on whether or not the program exits. I'm not currently understanding your point about it being confusing if the error message assumes otherwise. The program does continue, doing exactly what is said in the error message. I'm also not sure what sort of "ignore all errors" flag would even be useful to suppress this message.

[pcc]

Yes, technically the program does continue but my point is that won't matter to the user because the output file will not be generated if the program is implemented correctly. By "ignore all errors" flag I mean something like the --noinhibit-exec passed to lld, which would convert errors to warnings where possible. I'm not sure if this error would actually be converted to a warning by that flag, though.

[boomanaiden154]

Why would the output file not be generated correctly? I think at this point it's just a difference in interpretation over what emitError should do. It doesn't exit the program, it simply outputs to STDOUT/STDERR that an error occurred, and then the program continues on as normal (with the default eviction advisor in this case).