Skip to content

Conversation

@artagnon
Copy link
Contributor

There is an uncovered codepath in InstCombineVectorOps, where a new GEP instruction is created via buildNew. Fix this coverage hole.

artagnon added 2 commits May 13, 2025 13:55
There is an uncovered codepath in InstCombineVectorOps, where a new GEP
instruction is created via buildNew. Fix this coverage hole.
@artagnon artagnon requested review from dtcxzyw, goldsteinn and nikic May 13, 2025 13:09
@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels May 13, 2025
@llvmbot
Copy link
Member

llvmbot commented May 13, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Ramkumar Ramachandra (artagnon)

Changes

There is an uncovered codepath in InstCombineVectorOps, where a new GEP instruction is created via buildNew. Fix this coverage hole.


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

1 Files Affected:

  • (modified) llvm/test/Transforms/InstCombine/vec_shuffle.ll (+13)
diff --git a/llvm/test/Transforms/InstCombine/vec_shuffle.ll b/llvm/test/Transforms/InstCombine/vec_shuffle.ll
index dd9fab794917f..fa34a42714c46 100644
--- a/llvm/test/Transforms/InstCombine/vec_shuffle.ll
+++ b/llvm/test/Transforms/InstCombine/vec_shuffle.ll
@@ -248,6 +248,19 @@ define <2 x i8> @test13a(i8 %x1, i8 %x2) {
   ret <2 x i8> %D
 }
 
+define <1 x ptr> @shuffle_gep(ptr %x1, ptr %x2) {
+; CHECK-LABEL: @shuffle_gep(
+; CHECK-NEXT:    [[TMP1:%.*]] = insertelement <1 x ptr> poison, ptr [[X2:%.*]], i64 0
+; CHECK-NEXT:    [[RET:%.*]] = getelementptr i8, <1 x ptr> [[TMP1]], i64 5
+; CHECK-NEXT:    ret <1 x ptr> [[RET]]
+;
+  %ins.1 = insertelement <2 x ptr> poison, ptr %x1, i32 0
+  %ins.2 = insertelement <2 x ptr> %ins.1, ptr %x2, i32 1
+  %gep = getelementptr i8, <2 x ptr> %ins.2, i64 5
+  %ret = shufflevector <2 x ptr> %gep, <2 x ptr> poison, <1 x i32> <i32 1>
+  ret <1 x ptr> %ret
+}
+
 ; Increasing length of vector ops is not a good canonicalization.
 
 define <3 x i32> @add_wider(i32 %y, i32 %z) {

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. Might be worth checking flag preservation as well?

@artagnon
Copy link
Contributor Author

LGTM. Might be worth checking flag preservation as well?

Yes, the flag preservation is only for inbounds at the moment. I have a follow-up fixing that with tests, downstream.

@artagnon artagnon merged commit 5c084a1 into llvm:main May 22, 2025
14 checks passed
@artagnon artagnon deleted the ic-shufgep-coverage branch May 22, 2025 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants