This is an archive of the discontinued LLVM Phabricator instance.

Differential D143681

[llvm][test] restrict 2 GVN tests to just test GVN (NFC)
ClosedPublic

Authored by nickdesaulniers on Feb 9 2023, 2:22 PM.

Details

Reviewers
kcc
nikic
Commits
rGc062e97acfba: [llvm][test] restrict 2 GVN tests to just test GVN (NFC)

Diff Detail

Event Timeline

nickdesaulniers created this revision.Feb 9 2023, 2:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 9 2023, 2:22 PM
Herald added a subscriber: kmitropoulou. · View Herald Transcript
nickdesaulniers requested review of this revision.Feb 9 2023, 2:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 9 2023, 2:22 PM
Herald added a subscriber: llvm-commits. · View Herald Transcript
nickdesaulniers added reviewers: kcc, nikic.Feb 9 2023, 2:24 PM
Herald added a subscriber: StephenFan. · View Herald TranscriptFeb 9 2023, 2:24 PM
nickdesaulniers added a child revision: D143682: [llvm][test] convert one test to use opaque ptrs (NFC).Feb 9 2023, 2:24 PM
nickdesaulniers added inline comments.Feb 9 2023, 2:40 PM
llvm/test/Transforms/GVN/no_speculative_loads_with_asan.ll
6

Q: isn't %i8 based on gep ptr %i, 1 while %i3 is based on get ptr %i, 2? Aren't those different addresses? Or is it because it's a 16b load (so the two adjacent 8bits)?

nickdesaulniers added inline comments.Feb 9 2023, 2:56 PM
llvm/test/Transforms/NewGVN/no_speculative_loads_with_asan.ll
19–20

I think these two instructions show that this whole test is kind of pointless IMO.

It's a copy of llvm/test/Transforms/GVN/no_speculative_loads_with_asan.ll, but using newgvn rather than gvn (unless I messed something up by converting this from -O3.

The @TestNoAsan case is the same as the @TestAsan case, so it just shows that NewGVN cannot elide redundant non-local loads. Am I understanding correctly?

Harbormaster completed remote builds in B212893: Diff 496238.Feb 9 2023, 3:15 PM
nikic added inline comments.Feb 14 2023, 2:07 AM
llvm/test/Transforms/GVN/no_speculative_loads_with_asan.ll
6

%i8 is an out of bounds load which can be folded to poison (which then allows eliding the phi).

llvm/test/Transforms/NewGVN/no_speculative_loads_with_asan.ll
19–20

Yes, load folding in NewGVN is incomplete. Your new test it fine (the previous one was broken, because it did not actually test newgvn).

nickdesaulniers updated this revision to Diff 497359.Feb 14 2023, 9:22 AM
  • update comment in test as per @nikic
nickdesaulniers marked 2 inline comments as done.Feb 14 2023, 9:22 AM
Harbormaster completed remote builds in B213687: Diff 497359.Feb 14 2023, 10:25 AM
nikic accepted this revision.Feb 14 2023, 11:14 AM

LG

This revision is now accepted and ready to land.Feb 14 2023, 11:14 AM
Closed by commit rGc062e97acfba: [llvm][test] restrict 2 GVN tests to just test GVN (NFC) (authored by nickdesaulniers). · Explain WhyFeb 14 2023, 1:31 PM
This revision was automatically updated to reflect the committed changes.
nickdesaulniers added a commit: rGc062e97acfba: [llvm][test] restrict 2 GVN tests to just test GVN (NFC).

Revision Contents

PathSize
llvm/
test/
Transforms/
GVN/
18 lines
NewGVN/
24 lines

Diff 496238

llvm/test/Transforms/GVN/no_speculative_loads_with_asan.ll

; NOTE: Assertions have been autogenerated by utils/update_test_checks.py ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt -O3 -S %s | FileCheck %s ; RUN: opt -passes=gvn -S %s | FileCheck %s
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
declare noalias ptr @_Znam(i64) #1 declare noalias ptr @_Znam(i64) #1
; load of %i8 is redundant wrt. load of %i3.
nickdesaulniersAuthorUnsubmitted
Done
ReplyInline Actions

Q: isn't %i8 based on gep ptr %i, 1 while %i3 is based on get ptr %i, 2? Aren't those different addresses? Or is it because it's a 16b load (so the two adjacent 8bits)?

nickdesaulniers: Q: isn't `%i8` based on `gep ptr %i, 1` while `%i3` is based on `get ptr %i, 2`? Aren't those…
nikicUnsubmitted
Done
ReplyInline Actions

%i8 is an out of bounds load which can be folded to poison (which then allows eliding the phi).

nikic: `%i8` is an out of bounds load which can be folded to poison (which then allows eliding the…
define i32 @TestNoAsan() { define i32 @TestNoAsan() {
; CHECK-LABEL: @TestNoAsan( ; CHECK-LABEL: @TestNoAsan(
; CHECK-NEXT: bb: ; CHECK-NEXT: bb:
; CHECK-NEXT: [[I:%.*]] = tail call noalias ptr @_Znam(i64 2)
; CHECK-NEXT: [[I1:%.*]] = getelementptr inbounds i8, ptr [[I]], i64 1
; CHECK-NEXT: store i8 0, ptr [[I1]], align 1
; CHECK-NEXT: store i8 0, ptr [[I]], align 1
; CHECK-NEXT: [[I3:%.*]] = load i16, ptr [[I]], align 4
; CHECK-NEXT: [[I4:%.*]] = icmp eq i16 [[I3]], 0
; CHECK-NEXT: br i1 [[I4]], label [[BB10:%.*]], label [[BB5:%.*]]
; CHECK: bb5:
; CHECK-NEXT: [[I6:%.*]] = getelementptr inbounds i8, ptr [[I]], i64 2
; CHECK-NEXT: br label [[BB10]]
; CHECK: bb10:
; CHECK-NEXT: ret i32 0 ; CHECK-NEXT: ret i32 0
; ;
bb: bb:
%i = tail call noalias ptr @_Znam(i64 2) %i = tail call noalias ptr @_Znam(i64 2)
%i1 = getelementptr inbounds i8, ptr %i, i64 1 %i1 = getelementptr inbounds i8, ptr %i, i64 1
store i8 0, ptr %i1, align 1 store i8 0, ptr %i1, align 1
store i8 0, ptr %i, align 1 store i8 0, ptr %i, align 1
%i3 = load i16, ptr %i, align 4 %i3 = load i16, ptr %i, align 4
Show All 9 Lines
bb10: ; preds = %bb5, %bb bb10: ; preds = %bb5, %bb
%i11 = phi i32 [ %i9, %bb5 ], [ 0, %bb ] %i11 = phi i32 [ %i9, %bb5 ], [ 0, %bb ]
ret i32 %i11 ret i32 %i11
} }
define i32 @TestAsan() sanitize_address { define i32 @TestAsan() sanitize_address {
; CHECK-LABEL: @TestAsan( ; CHECK-LABEL: @TestAsan(
; CHECK-NEXT: bb: ; CHECK-NEXT: bb:
; CHECK-NEXT: [[I:%.*]] = tail call noalias dereferenceable_or_null(2) ptr @_Znam(i64 2) ; CHECK-NEXT: [[I:%.*]] = tail call noalias ptr @_Znam(i64 2)
; CHECK-NEXT: [[I1:%.*]] = getelementptr inbounds i8, ptr [[I]], i64 1 ; CHECK-NEXT: [[I1:%.*]] = getelementptr inbounds i8, ptr [[I]], i64 1
; CHECK-NEXT: store i8 0, ptr [[I1]], align 1 ; CHECK-NEXT: store i8 0, ptr [[I1]], align 1
; CHECK-NEXT: store i8 0, ptr [[I]], align 1 ; CHECK-NEXT: store i8 0, ptr [[I]], align 1
; CHECK-NEXT: [[I3:%.*]] = load i16, ptr [[I]], align 4 ; CHECK-NEXT: [[I3:%.*]] = load i16, ptr [[I]], align 4
; CHECK-NEXT: [[I4:%.*]] = icmp eq i16 [[I3]], 0 ; CHECK-NEXT: [[I4:%.*]] = icmp eq i16 [[I3]], 0
; CHECK-NEXT: br i1 [[I4]], label [[BB10:%.*]], label [[BB5:%.*]] ; CHECK-NEXT: br i1 [[I4]], label [[BB10:%.*]], label [[BB5:%.*]]
; CHECK: bb5: ; CHECK: bb5:
; CHECK-NEXT: [[I6:%.*]] = getelementptr inbounds i8, ptr [[I]], i64 2 ; CHECK-NEXT: [[I6:%.*]] = getelementptr inbounds i8, ptr [[I]], i64 2
Show All 22 Lines
bb10: ; preds = %bb5, %bb bb10: ; preds = %bb5, %bb
%i11 = phi i32 [ %i9, %bb5 ], [ 0, %bb ] %i11 = phi i32 [ %i9, %bb5 ], [ 0, %bb ]
ret i32 %i11 ret i32 %i11
} }
define i32 @TestHWAsan() sanitize_hwaddress { define i32 @TestHWAsan() sanitize_hwaddress {
; CHECK-LABEL: @TestHWAsan( ; CHECK-LABEL: @TestHWAsan(
; CHECK-NEXT: bb: ; CHECK-NEXT: bb:
; CHECK-NEXT: [[I:%.*]] = tail call noalias dereferenceable_or_null(2) ptr @_Znam(i64 2) ; CHECK-NEXT: [[I:%.*]] = tail call noalias ptr @_Znam(i64 2)
; CHECK-NEXT: [[I1:%.*]] = getelementptr inbounds i8, ptr [[I]], i64 1 ; CHECK-NEXT: [[I1:%.*]] = getelementptr inbounds i8, ptr [[I]], i64 1
; CHECK-NEXT: store i8 0, ptr [[I1]], align 1 ; CHECK-NEXT: store i8 0, ptr [[I1]], align 1
; CHECK-NEXT: store i8 0, ptr [[I]], align 1 ; CHECK-NEXT: store i8 0, ptr [[I]], align 1
; CHECK-NEXT: [[I3:%.*]] = load i16, ptr [[I]], align 4 ; CHECK-NEXT: [[I3:%.*]] = load i16, ptr [[I]], align 4
; CHECK-NEXT: [[I4:%.*]] = icmp eq i16 [[I3]], 0 ; CHECK-NEXT: [[I4:%.*]] = icmp eq i16 [[I3]], 0
; CHECK-NEXT: br i1 [[I4]], label [[BB10:%.*]], label [[BB5:%.*]] ; CHECK-NEXT: br i1 [[I4]], label [[BB10:%.*]], label [[BB5:%.*]]
; CHECK: bb5: ; CHECK: bb5:
; CHECK-NEXT: [[I6:%.*]] = getelementptr inbounds i8, ptr [[I]], i64 2 ; CHECK-NEXT: [[I6:%.*]] = getelementptr inbounds i8, ptr [[I]], i64 2
Show All 26 Lines

llvm/test/Transforms/NewGVN/no_speculative_loads_with_asan.ll

; NOTE: Assertions have been autogenerated by utils/update_test_checks.py ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt -opaque-pointers=0 -O3 -S %s | FileCheck %s ; RUN: opt -opaque-pointers=0 -passes=newgvn -S %s | FileCheck %s
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
declare noalias i8* @_Znam(i64) #1 declare noalias i8* @_Znam(i64) #1
define i32 @TestNoAsan() { define i32 @TestNoAsan() {
; CHECK-LABEL: @TestNoAsan( ; CHECK-LABEL: @TestNoAsan(
; CHECK-NEXT: ret i32 0 ; CHECK-NEXT: [[TMP1:%.*]] = tail call noalias i8* @_Znam(i64 2)
; CHECK-NEXT: [[TMP2:%.*]] = getelementptr inbounds i8, i8* [[TMP1]], i64 1
; CHECK-NEXT: store i8 0, i8* [[TMP2]], align 1
; CHECK-NEXT: store i8 0, i8* [[TMP1]], align 1
; CHECK-NEXT: [[TMP3:%.*]] = bitcast i8* [[TMP1]] to i16*
; CHECK-NEXT: [[TMP4:%.*]] = load i16, i16* [[TMP3]], align 4
; CHECK-NEXT: [[TMP5:%.*]] = icmp eq i16 [[TMP4]], 0
; CHECK-NEXT: br i1 [[TMP5]], label [[TMP11:%.*]], label [[TMP6:%.*]]
; CHECK: 6:
; CHECK-NEXT: [[TMP7:%.*]] = getelementptr inbounds i8, i8* [[TMP1]], i64 2
; CHECK-NEXT: [[TMP8:%.*]] = bitcast i8* [[TMP7]] to i16*
; CHECK-NEXT: [[TMP9:%.*]] = load i16, i16* [[TMP8]], align 2
; CHECK-NEXT: [[TMP10:%.*]] = sext i16 [[TMP9]] to i32
nickdesaulniersAuthorUnsubmitted
Done
ReplyInline Actions

I think these two instructions show that this whole test is kind of pointless IMO.

It's a copy of llvm/test/Transforms/GVN/no_speculative_loads_with_asan.ll, but using newgvn rather than gvn (unless I messed something up by converting this from -O3.

The @TestNoAsan case is the same as the @TestAsan case, so it just shows that NewGVN cannot elide redundant non-local loads. Am I understanding correctly?

nickdesaulniers: I think these two instructions show that this whole test is kind of pointless IMO. It's a copy…
nikicUnsubmitted
Done
ReplyInline Actions

Yes, load folding in NewGVN is incomplete. Your new test it fine (the previous one was broken, because it did not actually test newgvn).

nikic: Yes, load folding in NewGVN is incomplete. Your new test it fine (the previous one was broken…
; CHECK-NEXT: br label [[TMP11]]
; CHECK: 11:
; CHECK-NEXT: [[TMP12:%.*]] = phi i32 [ [[TMP10]], [[TMP6]] ], [ 0, [[TMP0:%.*]] ]
; CHECK-NEXT: ret i32 [[TMP12]]
; ;
%1 = tail call noalias i8* @_Znam(i64 2) %1 = tail call noalias i8* @_Znam(i64 2)
%2 = getelementptr inbounds i8, i8* %1, i64 1 %2 = getelementptr inbounds i8, i8* %1, i64 1
store i8 0, i8* %2, align 1 store i8 0, i8* %2, align 1
store i8 0, i8* %1, align 1 store i8 0, i8* %1, align 1
%3 = bitcast i8* %1 to i16* %3 = bitcast i8* %1 to i16*
%4 = load i16, i16* %3, align 4 %4 = load i16, i16* %3, align 4
%5 = icmp eq i16 %4, 0 %5 = icmp eq i16 %4, 0
br i1 %5, label %11, label %6 br i1 %5, label %11, label %6
; <label>:6 ; preds = %0 ; <label>:6 ; preds = %0
%7 = getelementptr inbounds i8, i8* %1, i64 2 %7 = getelementptr inbounds i8, i8* %1, i64 2
%8 = bitcast i8* %7 to i16* %8 = bitcast i8* %7 to i16*
%9 = load i16, i16* %8, align 2 %9 = load i16, i16* %8, align 2
%10 = sext i16 %9 to i32 %10 = sext i16 %9 to i32
br label %11 br label %11
; <label>:11 ; preds = %0, %6 ; <label>:11 ; preds = %0, %6
%12 = phi i32 [ %10, %6 ], [ 0, %0 ] %12 = phi i32 [ %10, %6 ], [ 0, %0 ]
ret i32 %12 ret i32 %12
} }
define i32 @TestAsan() sanitize_address { define i32 @TestAsan() sanitize_address {
; CHECK-LABEL: @TestAsan( ; CHECK-LABEL: @TestAsan(
; CHECK-NEXT: [[TMP1:%.*]] = tail call noalias dereferenceable_or_null(2) i8* @_Znam(i64 2) ; CHECK-NEXT: [[TMP1:%.*]] = tail call noalias i8* @_Znam(i64 2)
; CHECK-NEXT: [[TMP2:%.*]] = getelementptr inbounds i8, i8* [[TMP1]], i64 1 ; CHECK-NEXT: [[TMP2:%.*]] = getelementptr inbounds i8, i8* [[TMP1]], i64 1
; CHECK-NEXT: store i8 0, i8* [[TMP2]], align 1 ; CHECK-NEXT: store i8 0, i8* [[TMP2]], align 1
; CHECK-NEXT: store i8 0, i8* [[TMP1]], align 1 ; CHECK-NEXT: store i8 0, i8* [[TMP1]], align 1
; CHECK-NEXT: [[TMP3:%.*]] = bitcast i8* [[TMP1]] to i16* ; CHECK-NEXT: [[TMP3:%.*]] = bitcast i8* [[TMP1]] to i16*
; CHECK-NEXT: [[TMP4:%.*]] = load i16, i16* [[TMP3]], align 4 ; CHECK-NEXT: [[TMP4:%.*]] = load i16, i16* [[TMP3]], align 4
; CHECK-NEXT: [[TMP5:%.*]] = icmp eq i16 [[TMP4]], 0 ; CHECK-NEXT: [[TMP5:%.*]] = icmp eq i16 [[TMP4]], 0
; CHECK-NEXT: br i1 [[TMP5]], label [[TMP11:%.*]], label [[TMP6:%.*]] ; CHECK-NEXT: br i1 [[TMP5]], label [[TMP11:%.*]], label [[TMP6:%.*]]
; CHECK: 6: ; CHECK: 6:
Show All 24 Lines
; <label>:11 ; preds = %0, %6 ; <label>:11 ; preds = %0, %6
%12 = phi i32 [ %10, %6 ], [ 0, %0 ] %12 = phi i32 [ %10, %6 ], [ 0, %0 ]
ret i32 %12 ret i32 %12
} }
define i32 @TestHWAsan() sanitize_hwaddress { define i32 @TestHWAsan() sanitize_hwaddress {
; CHECK-LABEL: @TestHWAsan( ; CHECK-LABEL: @TestHWAsan(
; CHECK-NEXT: [[TMP1:%.*]] = tail call noalias dereferenceable_or_null(2) i8* @_Znam(i64 2) ; CHECK-NEXT: [[TMP1:%.*]] = tail call noalias i8* @_Znam(i64 2)
; CHECK-NEXT: [[TMP2:%.*]] = getelementptr inbounds i8, i8* [[TMP1]], i64 1 ; CHECK-NEXT: [[TMP2:%.*]] = getelementptr inbounds i8, i8* [[TMP1]], i64 1
; CHECK-NEXT: store i8 0, i8* [[TMP2]], align 1 ; CHECK-NEXT: store i8 0, i8* [[TMP2]], align 1
; CHECK-NEXT: store i8 0, i8* [[TMP1]], align 1 ; CHECK-NEXT: store i8 0, i8* [[TMP1]], align 1
; CHECK-NEXT: [[TMP3:%.*]] = bitcast i8* [[TMP1]] to i16* ; CHECK-NEXT: [[TMP3:%.*]] = bitcast i8* [[TMP1]] to i16*
; CHECK-NEXT: [[TMP4:%.*]] = load i16, i16* [[TMP3]], align 4 ; CHECK-NEXT: [[TMP4:%.*]] = load i16, i16* [[TMP3]], align 4
; CHECK-NEXT: [[TMP5:%.*]] = icmp eq i16 [[TMP4]], 0 ; CHECK-NEXT: [[TMP5:%.*]] = icmp eq i16 [[TMP4]], 0
; CHECK-NEXT: br i1 [[TMP5]], label [[TMP11:%.*]], label [[TMP6:%.*]] ; CHECK-NEXT: br i1 [[TMP5]], label [[TMP11:%.*]], label [[TMP6:%.*]]
; CHECK: 6: ; CHECK: 6:
Show All 29 Lines