[TableGen] Remove unhelpful error messages from PseudoLoweringEmitter. #135747
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
All of the notes using the location of ResultInst will just print the location inside of the PseudoInstExpansion class. There was one note using the location of DI->getDef(), but I don't think that one is particularly helpful either.
|
@llvm/pr-subscribers-tablegen Author: Craig Topper (topperc) ChangesAll of the notes using the location of ResultInst will just print the location inside of the PseudoInstExpansion class. There was one note using the location of DI->getDef(), but knowing where one of the two mismatched types is defined isn't helpful. The operand types need to be the same, so the mismatch message we already printed should be enough. Full diff: https://github.com/llvm/llvm-project/pull/135747.diff 1 Files Affected:
diff --git a/llvm/utils/TableGen/PseudoLoweringEmitter.cpp b/llvm/utils/TableGen/PseudoLoweringEmitter.cpp
index 56b9e499cb4aa..7f67c13c0bbbd 100644
--- a/llvm/utils/TableGen/PseudoLoweringEmitter.cpp
+++ b/llvm/utils/TableGen/PseudoLoweringEmitter.cpp
@@ -90,14 +90,12 @@ unsigned PseudoLoweringEmitter::addDagOperandMapping(
// problem.
// FIXME: We probably shouldn't ever get a non-zero BaseIdx here.
assert(BaseIdx == 0 && "Named subargument in pseudo expansion?!");
- if (DI->getDef() != Insn.Operands[BaseIdx + i].Rec) {
- PrintError(Rec, "In pseudo instruction '" + Rec->getName() +
- "', operand type '" + DI->getDef()->getName() +
- "' does not match expansion operand type '" +
- Insn.Operands[BaseIdx + i].Rec->getName() + "'");
- PrintFatalNote(DI->getDef(),
- "Value was assigned at the following location:");
- }
+ if (DI->getDef() != Insn.Operands[BaseIdx + i].Rec)
+ PrintFatalError(Rec, "In pseudo instruction '" + Rec->getName() +
+ "', operand type '" + DI->getDef()->getName() +
+ "' does not match expansion operand type '" +
+ Insn.Operands[BaseIdx + i].Rec->getName() +
+ "'");
// Source operand maps to destination operand. The Data element
// will be filled in later, just set the Kind for now. Do it
// for each corresponding MachineInstr operand, not just the first.
@@ -138,38 +136,26 @@ void PseudoLoweringEmitter::evaluateExpansion(const Record *Rec) {
LLVM_DEBUG(dbgs() << " Result: " << *Dag << "\n");
const DefInit *OpDef = dyn_cast<DefInit>(Dag->getOperator());
- if (!OpDef) {
- PrintError(Rec, "In pseudo instruction '" + Rec->getName() +
- "', result operator is not a record");
- PrintFatalNote(Rec->getValue("ResultInst"),
- "Result was assigned at the following location:");
- }
+ if (!OpDef)
+ PrintFatalError(Rec, "In pseudo instruction '" + Rec->getName() +
+ "', result operator is not a record");
const Record *Operator = OpDef->getDef();
- if (!Operator->isSubClassOf("Instruction")) {
- PrintError(Rec, "In pseudo instruction '" + Rec->getName() +
- "', result operator '" + Operator->getName() +
- "' is not an instruction");
- PrintFatalNote(Rec->getValue("ResultInst"),
- "Result was assigned at the following location:");
- }
+ if (!Operator->isSubClassOf("Instruction"))
+ PrintFatalError(Rec, "In pseudo instruction '" + Rec->getName() +
+ "', result operator '" + Operator->getName() +
+ "' is not an instruction");
CodeGenInstruction Insn(Operator);
- if (Insn.isCodeGenOnly || Insn.isPseudo) {
- PrintError(Rec, "In pseudo instruction '" + Rec->getName() +
- "', result operator '" + Operator->getName() +
- "' cannot be a pseudo instruction");
- PrintFatalNote(Rec->getValue("ResultInst"),
- "Result was assigned at the following location:");
- }
+ if (Insn.isCodeGenOnly || Insn.isPseudo)
+ PrintFatalError(Rec, "In pseudo instruction '" + Rec->getName() +
+ "', result operator '" + Operator->getName() +
+ "' cannot be a pseudo instruction");
- if (Insn.Operands.size() != Dag->getNumArgs()) {
- PrintError(Rec, "In pseudo instruction '" + Rec->getName() +
- "', result operator '" + Operator->getName() +
- "' has the wrong number of operands");
- PrintFatalNote(Rec->getValue("ResultInst"),
- "Result was assigned at the following location:");
- }
+ if (Insn.Operands.size() != Dag->getNumArgs())
+ PrintFatalError(Rec, "In pseudo instruction '" + Rec->getName() +
+ "', result operator '" + Operator->getName() +
+ "' has the wrong number of operands");
unsigned NumMIOperands = 0;
for (const auto &Op : Insn.Operands)
@@ -202,13 +188,10 @@ void PseudoLoweringEmitter::evaluateExpansion(const Record *Rec) {
continue;
StringMap<unsigned>::iterator SourceOp =
SourceOperands.find(Dag->getArgNameStr(i));
- if (SourceOp == SourceOperands.end()) {
- PrintError(Rec, "In pseudo instruction '" + Rec->getName() +
- "', output operand '" + Dag->getArgNameStr(i) +
- "' has no matching source operand");
- PrintFatalNote(Rec->getValue("ResultInst"),
- "Value was assigned at the following location:");
- }
+ if (SourceOp == SourceOperands.end())
+ PrintFatalError(Rec, "In pseudo instruction '" + Rec->getName() +
+ "', output operand '" + Dag->getArgNameStr(i) +
+ "' has no matching source operand");
// Map the source operand to the destination operand index for each
// MachineInstr operand.
for (unsigned I = 0, E = Insn.Operands[i].MINumOperands; I != E; ++I)
|
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/66/builds/12603 Here is the relevant piece of the build log for the reference |
var-const
pushed a commit
to ldionne/llvm-project
that referenced
this pull request
Apr 17, 2025
llvm#135747) All of the notes using the location of ResultInst will just print the location inside of the PseudoInstExpansion class. There was one note using the location of DI->getDef(), but knowing where one of the two mismatched types is defined isn't helpful. The operand types need to be the same, so the mismatch message we already printed should be enough.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
All of the notes using the location of ResultInst will just print the location inside of the PseudoInstExpansion class.
There was one note using the location of DI->getDef(), but knowing where one of the two mismatched types is defined isn't helpful. The operand types need to be the same, so the mismatch message we already printed should be enough.