Skip to content

[SelectionDAGBuilder] Replace asserts inside LLVM_DEBUG. NFC#199748

Open
davemgreen wants to merge 1 commit into
llvm:mainfrom
davemgreen:gh-dag-debugassert
Open

[SelectionDAGBuilder] Replace asserts inside LLVM_DEBUG. NFC#199748
davemgreen wants to merge 1 commit into
llvm:mainfrom
davemgreen:gh-dag-debugassert

Conversation

@davemgreen
Copy link
Copy Markdown
Contributor

These assert were inside an LLVM_DEBUG macro, meaning they were very rarely if ever tested. The second "LowerFormalArguments emitted a value with the wrong type!" assert would fire in a number of tests so has been removed. The other was replaced with an all_of assert.

Noticed when looking at #198107 / #199412.

These assert were inside an LLVM_DEBUG macro, meaning they were very rarely if
ever tested. The second "LowerFormalArguments emitted a value with the wrong
type!" assert would fire in a number of tests so has been removed. The other
was replaced with an all_of assert.
@llvmorg-github-actions llvmorg-github-actions Bot added the llvm:SelectionDAG SelectionDAGISel as well label May 26, 2026
@llvmorg-github-actions
Copy link
Copy Markdown

@llvm/pr-subscribers-llvm-selectiondag

Author: David Green (davemgreen)

Changes

These assert were inside an LLVM_DEBUG macro, meaning they were very rarely if ever tested. The second "LowerFormalArguments emitted a value with the wrong type!" assert would fire in a number of tests so has been removed. The other was replaced with an all_of assert.

Noticed when looking at #198107 / #199412.


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

1 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (+2-8)
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 579bff7d3ab60..05c75c484889e 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -12097,14 +12097,8 @@ void SelectionDAGISel::LowerArguments(const Function &F) {
          "LowerFormalArguments didn't return a valid chain!");
   assert(InVals.size() == Ins.size() &&
          "LowerFormalArguments didn't emit the correct number of values!");
-  LLVM_DEBUG({
-    for (unsigned i = 0, e = Ins.size(); i != e; ++i) {
-      assert(InVals[i].getNode() &&
-             "LowerFormalArguments emitted a null value!");
-      assert(EVT(Ins[i].VT) == InVals[i].getValueType() &&
-             "LowerFormalArguments emitted a value with the wrong type!");
-    }
-  });
+  assert(all_of(InVals, [](SDValue InVal) { return InVal.getNode(); }) &&
+         "LowerFormalArguments emitted a null value!");
 
   // Update the DAG with the new chain value resulting from argument lowering.
   DAG.setRoot(NewRoot);

Copy link
Copy Markdown
Contributor

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

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

LGTM

I was curious, so I dug a bit for other instances of this pattern. Fortunately it seems to be pretty rare. Found some use in flang/lib/Optimizer/Transforms/FIRToMemRef.cpp, mlir/lib/Transforms/Utils/DialectConversion.cpp . (Not a comprehensive survey.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:SelectionDAG SelectionDAGISel as well

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants