Skip to content

[InstCombine] Add test for freeze of GEP with recurrence offset (NFC) #145541

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

c-rhodes
Copy link
Collaborator

The freeze should be pushed through the GEP to the ptr like in: https://godbolt.org/z/jrcozT8rz

@c-rhodes c-rhodes requested review from arsenm, nikic and david-arm June 24, 2025 16:11
@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels Jun 24, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 24, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Cullen Rhodes (c-rhodes)

Changes

The freeze should be pushed through the GEP to the ptr like in: https://godbolt.org/z/jrcozT8rz


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

1 Files Affected:

  • (modified) llvm/test/Transforms/InstCombine/freeze.ll (+33)
diff --git a/llvm/test/Transforms/InstCombine/freeze.ll b/llvm/test/Transforms/InstCombine/freeze.ll
index 8875ce1c566f3..50483d8e41ada 100644
--- a/llvm/test/Transforms/InstCombine/freeze.ll
+++ b/llvm/test/Transforms/InstCombine/freeze.ll
@@ -877,6 +877,39 @@ exit:                                             ; preds = %loop
   ret void
 }
 
+; The recurrence for the GEP offset can't produce undef so the freeze should be
+; pushed through to the ptr, but this is not currently supported.
+define void @fold_phi_gep_phi_offset(ptr %init, ptr %end, i64 noundef %n) {
+; CHECK-LABEL: @fold_phi_phi_gep(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[LOOP:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    [[I:%.*]] = phi ptr [ [[INIT:%.*]], [[ENTRY:%.*]] ], [ [[I_NEXT_FR:%.*]], [[LOOP]] ]
+; CHECK-NEXT:    [[OFF:%.*]] = phi i64 [ [[N:%.*]], [[ENTRY]] ], [ [[OFF_NEXT:%.*]], [[LOOP]] ]
+; CHECK-NEXT:    [[OFF_NEXT]] = shl i64 [[OFF]], 3
+; CHECK-NEXT:    [[I_NEXT:%.*]] = getelementptr i8, ptr [[I]], i64 [[OFF_NEXT]]
+; CHECK-NEXT:    [[I_NEXT_FR]] = freeze ptr [[I_NEXT]]
+; CHECK-NEXT:    [[COND:%.*]] = icmp eq ptr [[I_NEXT_FR]], [[END:%.*]]
+; CHECK-NEXT:    br i1 [[COND]], label [[LOOP]], label [[EXIT:%.*]]
+; CHECK:       exit:
+; CHECK-NEXT:    ret void
+;
+entry:
+  br label %loop
+
+loop:                                             ; preds = %loop, %entry
+  %i = phi ptr [ %init, %entry ], [ %i.next.fr, %loop ]
+  %off = phi i64 [ %n, %entry ], [ %off.next, %loop ]
+  %off.next = shl i64 %off, 3
+  %i.next = getelementptr i8, ptr %i, i64 %off.next
+  %i.next.fr = freeze ptr %i.next
+  %cond = icmp eq ptr %i.next.fr, %end
+  br i1 %cond, label %loop, label %exit
+
+exit:                                             ; preds = %loop
+  ret void
+}
+
 define void @fold_phi_multiple_insts(i32 %init, i32 %n) {
 ; CHECK-LABEL: @fold_phi_multiple_insts(
 ; CHECK-NEXT:  entry:

Copy link

⚠️ undef deprecator found issues in your code. ⚠️

You can test this locally with the following command:
git diff -U0 --pickaxe-regex -S '([^a-zA-Z0-9#_-]undef[^a-zA-Z0-9_-]|UndefValue::get)' 'HEAD~1' HEAD llvm/test/Transforms/InstCombine/freeze.ll

The following files introduce new uses of undef:

  • llvm/test/Transforms/InstCombine/freeze.ll

Undef is now deprecated and should only be used in the rare cases where no replacement is possible. For example, a load of uninitialized memory yields undef. You should use poison values for placeholders instead.

In tests, avoid using undef and having tests that trigger undefined behavior. If you need an operand with some unimportant value, you can add a new argument to the function and use that instead.

For example, this is considered a bad practice:

define void @fn() {
  ...
  br i1 undef, ...
}

Please use the following instead:

define void @fn(i1 %cond) {
  ...
  br i1 %cond, ...
}

Please refer to the Undefined Behavior Manual for more information.

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.

2 participants