Skip to content

[SROA] Allow as zext<i1> index when unfolding GEP select #146929

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

AlexMaclean
Copy link
Member

A zero-extension from an i1 is equivalent to a select with constant 0 and 1 values. Add this case when rewriting gep(select) -> select(gep) to expose more opportunities for SROA.

@AlexMaclean AlexMaclean requested a review from nikic July 3, 2025 17:27
@AlexMaclean AlexMaclean self-assigned this Jul 3, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 3, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Alex MacLean (AlexMaclean)

Changes

A zero-extension from an i1 is equivalent to a select with constant 0 and 1 values. Add this case when rewriting gep(select) -> select(gep) to expose more opportunities for SROA.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/SROA.cpp (+23-6)
  • (modified) llvm/test/Transforms/SROA/select-gep.ll (+18)
diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp b/llvm/lib/Transforms/Scalar/SROA.cpp
index 42d1d9a437bb2..dd2e83ca5ecec 100644
--- a/llvm/lib/Transforms/Scalar/SROA.cpp
+++ b/llvm/lib/Transforms/Scalar/SROA.cpp
@@ -4070,18 +4070,27 @@ class AggLoadStoreRewriter : public InstVisitor<AggLoadStoreRewriter, bool> {
   //   => select cond, gep(ptr1, idx), gep(ptr2, idx)
   // and  gep ptr, (select cond, idx1, idx2)
   //   => select cond, gep(ptr, idx1), gep(ptr, idx2)
+  // We also allow for i1 zext indices, which are equivalent to selects.
   bool unfoldGEPSelect(GetElementPtrInst &GEPI) {
     // Check whether the GEP has exactly one select operand and all indices
     // will become constant after the transform.
-    SelectInst *Sel = dyn_cast<SelectInst>(GEPI.getPointerOperand());
+    Instruction *Sel = dyn_cast<SelectInst>(GEPI.getPointerOperand());
     for (Value *Op : GEPI.indices()) {
       if (auto *SI = dyn_cast<SelectInst>(Op)) {
         if (Sel)
           return false;
 
         Sel = SI;
-        if (!isa<ConstantInt>(Sel->getTrueValue()) ||
-            !isa<ConstantInt>(Sel->getFalseValue()))
+        if (!isa<ConstantInt>(SI->getTrueValue()) ||
+            !isa<ConstantInt>(SI->getFalseValue()))
+          return false;
+        continue;
+      }
+      if (auto *ZI = dyn_cast<ZExtInst>(Op)) {
+        if (Sel)
+          return false;
+        Sel = ZI;
+        if (!ZI->getSrcTy()->isIntegerTy(1))
           return false;
         continue;
       }
@@ -4107,8 +4116,16 @@ class AggLoadStoreRewriter : public InstVisitor<AggLoadStoreRewriter, bool> {
       return NewOps;
     };
 
-    Value *True = Sel->getTrueValue();
-    Value *False = Sel->getFalseValue();
+    Value *Cond, *True, *False;
+    if (auto *SI = dyn_cast<SelectInst>(Sel)) {
+      Cond = SI->getCondition();
+      True = SI->getTrueValue();
+      False = SI->getFalseValue();
+    } else {
+      Cond = Sel->getOperand(0);
+      True = ConstantInt::get(Sel->getType(), 1);
+      False = ConstantInt::get(Sel->getType(), 0);
+    }
     SmallVector<Value *> TrueOps = GetNewOps(True);
     SmallVector<Value *> FalseOps = GetNewOps(False);
 
@@ -4123,7 +4140,7 @@ class AggLoadStoreRewriter : public InstVisitor<AggLoadStoreRewriter, bool> {
         IRB.CreateGEP(Ty, FalseOps[0], ArrayRef(FalseOps).drop_front(),
                       False->getName() + ".sroa.gep", NW);
 
-    Value *NSel = IRB.CreateSelect(Sel->getCondition(), NTrue, NFalse,
+    Value *NSel = IRB.CreateSelect(Cond, NTrue, NFalse,
                                    Sel->getName() + ".sroa.sel");
     Visited.erase(&GEPI);
     GEPI.replaceAllUsesWith(NSel);
diff --git a/llvm/test/Transforms/SROA/select-gep.ll b/llvm/test/Transforms/SROA/select-gep.ll
index 1342a2ca4ea2b..56c2b2fd87bd8 100644
--- a/llvm/test/Transforms/SROA/select-gep.ll
+++ b/llvm/test/Transforms/SROA/select-gep.ll
@@ -201,6 +201,24 @@ define i32 @test_select_idx_mem2reg(i1 %c) {
   ret i32 %res
 }
 
+; Test gep with a select-like zext index unfolding on an alloca that is
+; splittable and promotable.
+define i64 @test_select_like_zext_idx_mem2reg(i1 %c) {
+; CHECK-LABEL: @test_select_like_zext_idx_mem2reg(
+; CHECK-NEXT:    [[IDX:%.*]] = zext i1 [[C:%.*]] to i64
+; CHECK-NEXT:    [[RES:%.*]] = select i1 [[C]], i64 2, i64 1
+; CHECK-NEXT:    ret i64 [[RES]]
+;
+  %alloca = alloca [2 x i64], align 8
+  store i64 1, ptr %alloca
+  %gep1 = getelementptr inbounds i64, ptr %alloca, i64 1
+  store i64 2, ptr %gep1
+  %idx = zext i1 %c to i64
+  %gep2 = getelementptr inbounds i64, ptr %alloca, i64 %idx
+  %res = load i64, ptr %gep2
+  ret i64 %res
+}
+
 ; Test gep of index select unfolding on an alloca that escaped, and as such
 ; is not splittable or promotable.
 ; FIXME: Ideally, no transform would take place in this case.

Copy link

github-actions bot commented Jul 3, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@AlexMaclean AlexMaclean force-pushed the dev/amaclean/upstream/sroa-i1-zext branch from 60fd6bb to 496249e Compare July 3, 2025 17:40
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

@AlexMaclean AlexMaclean force-pushed the dev/amaclean/upstream/sroa-i1-zext branch from 496249e to 83d8478 Compare July 4, 2025 03:25
@AlexMaclean AlexMaclean merged commit 0008af8 into llvm:main Jul 4, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants