[InstCombine] Add test for freeze of GEP with recurrence offset (NFC) #145541
Conversation
The freeze should be pushed through the GEP to the ptr like in: https://godbolt.org/z/jrcozT8rz
llvmbot
added
llvm:instcombine
|
@llvm/pr-subscribers-llvm-transforms Author: Cullen Rhodes (c-rhodes) ChangesThe 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:
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:
|
|
✅ With the latest revision this PR passed the undef deprecator. |
|
the undef checker is firing because of undef in a comment, is it ok to keep this or should i use something else? |
| ret void | ||
| } | ||
|
|
||
| ; GEP can produce poison, check freeze isn't moved. |
There was a problem hiding this comment.
If I understand correctly the freeze could be moved here too if we strip inbounds from the gep and push the freeze up to the pointer %i? The offset %off.next is still guaranteed not to be poison or undef I think.
There was a problem hiding this comment.
If I understand correctly the freeze could be moved here too if we strip inbounds from the gep and push the freeze up to the pointer %i?
Yes I think so.
The offset %off.next is still guaranteed not to be poison or undef I think.
I don't believe so, %n needs to be marked noundef for that? I think this example is what you're referring to: https://godbolt.org/z/e8rGsz66W. Whereas Nikita wanted a test where the GEP can produce poison.
There was a problem hiding this comment.
Both variants probably have value. One requires just stripping poison, the other requires freezing an additional value.
There was a problem hiding this comment.
ok, I've also added a test for the inbounds GEP + noundef offset variant
|
@c-rhodes is on holiday for a while, but given this patch is only adding tests I figured it should ok for me to merge on his behalf. |
…llvm#145541) The freeze should be pushed through the GEP to the ptr like in: https://godbolt.org/z/jrcozT8rz
…llvm#145541) The freeze should be pushed through the GEP to the ptr like in: https://godbolt.org/z/jrcozT8rz
The freeze should be pushed through the GEP to the ptr like in: https://godbolt.org/z/jrcozT8rz