[NFC] Use cast instead of dyn_cast for Src and Dst vec types in VecCombine folding #134432
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
SrcVecTy and DstVecTy are used without a null check, and originate from a dyn_cast. This patch adds an assertion that these values are non-null before they are used/dereferenced.
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-vectorizers Author: None (calebwat) ChangesSrcVecTy and DstVecTy are used without a null check, and originate from a dyn_cast. This patch adds an assertion that these values are non-null before they are used/dereferenced. Full diff: https://github.com/llvm/llvm-project/pull/134432.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
index 4bfe41a5ed00d..fc04266cac51b 100644
--- a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
+++ b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
@@ -2052,6 +2052,9 @@ bool VectorCombine::foldShuffleOfSelects(Instruction &I) {
auto *SrcVecTy = dyn_cast<FixedVectorType>(T1->getType());
auto *DstVecTy = dyn_cast<FixedVectorType>(I.getType());
+ assert(SrcVecTy && DstVecTy &&
+ "Expected shuffle/select vector types to be defined");
+
auto SK = TargetTransformInfo::SK_PermuteTwoSrc;
auto SelOp = Instruction::Select;
InstructionCost OldCost = TTI.getCmpSelInstrCost(
|
shiltian
reviewed
Apr 4, 2025
| @@ -2052,6 +2052,9 @@ bool VectorCombine::foldShuffleOfSelects(Instruction &I) { | |||
|
|
|||
| auto *SrcVecTy = dyn_cast<FixedVectorType>(T1->getType()); | |||
| auto *DstVecTy = dyn_cast<FixedVectorType>(I.getType()); | |||
| assert(SrcVecTy && DstVecTy && | |||
There was a problem hiding this comment.
If they don't expect to be nullptr, a cast would be more appropriate.
There was a problem hiding this comment.
yes - replace those 2 dyn_cast with cast - the previous dyn_cast checks for the select condition operands should be ensuring FixedVectorType for everything
There was a problem hiding this comment.
Good points, thank you! I've updated the PR accordingly.
calebwat
changed the title
var-const
pushed a commit
to ldionne/llvm-project
that referenced
this pull request
Apr 17, 2025
…mbine folding (llvm#134432) SrcVecTy and DstVecTy are used without a null check, and originate from a dyn_cast. This patch adjusts this to use a fixed cast, since it is not checked for null before use otherwise, but is semantically guaranteed from previous checks.
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.
SrcVecTy and DstVecTy are used without a null check, and originate from a dyn_cast. This patch adjusts this to use a fixed cast, since it is not checked for null before use otherwise, but is semantically guaranteed from previous checks.