[GVN] Add MemorySSA checks in tests 1/N #130261

madhur13490 · 2025-03-07T08:47:57Z

There is no test coverage for MemorySSA for GVN tests. We should keep testing MemorySSA if we eventually want to make it the default.

llvmbot · 2025-03-07T08:48:30Z

@llvm/pr-subscribers-llvm-transforms

Author: Madhur Amilkanthwar (madhur13490)

Changes

There is no test coverage for MemorySSA for GVN tests. We should keep testing MemorySSA if we eventually want to make it the default.

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

4 Files Affected:

(modified) llvm/test/Transforms/GVN/assume.ll (+26)
(modified) llvm/test/Transforms/GVN/basic.ll (+12-6)
(modified) llvm/test/Transforms/GVN/nonescaping.ll (+16)
(modified) llvm/test/Transforms/GVN/pr14166.ll (+20-7)

diff --git a/llvm/test/Transforms/GVN/assume.ll b/llvm/test/Transforms/GVN/assume.ll
index 6cb4c871750d6..a1c485108622e 100644
--- a/llvm/test/Transforms/GVN/assume.ll
+++ b/llvm/test/Transforms/GVN/assume.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -passes=gvn -verify-analysis-invalidation -S | FileCheck %s
+; RUN: opt < %s -passes='require<memoryssa>,gvn' -verify-analysis-invalidation -S | FileCheck --check-prefix=CHECK-MSSA %s
 
 declare void @llvm.assume(i1)
 declare void @use(i1)
@@ -7,6 +8,9 @@ declare void @use(i1)
 define void @assume_true() {
 ; CHECK-LABEL: @assume_true(
 ; CHECK-NEXT:    ret void
+;
+; CHECK-MSSA-LABEL: @assume_true(
+; CHECK-MSSA-NEXT:    ret void
 ;
   call void @llvm.assume(i1 true)
   ret void
@@ -16,6 +20,10 @@ define void @assume_false() {
 ; CHECK-LABEL: @assume_false(
 ; CHECK-NEXT:    store i8 poison, ptr null, align 1
 ; CHECK-NEXT:    ret void
+;
+; CHECK-MSSA-LABEL: @assume_false(
+; CHECK-MSSA-NEXT:    store i8 poison, ptr null, align 1
+; CHECK-MSSA-NEXT:    ret void
 ;
   call void @llvm.assume(i1 false)
   ret void
@@ -26,6 +34,11 @@ define void @assume_arg(i1 %x) {
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[X:%.*]])
 ; CHECK-NEXT:    call void @use(i1 true)
 ; CHECK-NEXT:    ret void
+;
+; CHECK-MSSA-LABEL: @assume_arg(
+; CHECK-MSSA-NEXT:    call void @llvm.assume(i1 [[X:%.*]])
+; CHECK-MSSA-NEXT:    call void @use(i1 true)
+; CHECK-MSSA-NEXT:    ret void
 ;
   call void @llvm.assume(i1 %x)
   call void @use(i1 %x)
@@ -38,6 +51,12 @@ define void @assume_not_arg(i1 %x) {
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[XOR]])
 ; CHECK-NEXT:    call void @use(i1 false)
 ; CHECK-NEXT:    ret void
+;
+; CHECK-MSSA-LABEL: @assume_not_arg(
+; CHECK-MSSA-NEXT:    [[XOR:%.*]] = xor i1 [[X:%.*]], true
+; CHECK-MSSA-NEXT:    call void @llvm.assume(i1 [[XOR]])
+; CHECK-MSSA-NEXT:    call void @use(i1 false)
+; CHECK-MSSA-NEXT:    ret void
 ;
   %xor = xor i1 %x, true
   call void @llvm.assume(i1 %xor)
@@ -52,6 +71,13 @@ define void @pr47496(i8 %x) {
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[XOR]])
 ; CHECK-NEXT:    call void @use(i1 false)
 ; CHECK-NEXT:    ret void
+;
+; CHECK-MSSA-LABEL: @pr47496(
+; CHECK-MSSA-NEXT:    [[CMP:%.*]] = icmp slt i8 [[X:%.*]], 0
+; CHECK-MSSA-NEXT:    [[XOR:%.*]] = xor i1 [[CMP]], true
+; CHECK-MSSA-NEXT:    call void @llvm.assume(i1 [[XOR]])
+; CHECK-MSSA-NEXT:    call void @use(i1 false)
+; CHECK-MSSA-NEXT:    ret void
 ;
   %cmp = icmp slt i8 %x, 0
   %xor = xor i1 %cmp, true
diff --git a/llvm/test/Transforms/GVN/basic.ll b/llvm/test/Transforms/GVN/basic.ll
index 09155bf4cf2fd..b6dfeab85241c 100644
--- a/llvm/test/Transforms/GVN/basic.ll
+++ b/llvm/test/Transforms/GVN/basic.ll
@@ -1,15 +1,21 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
 ; RUN: opt < %s -passes=gvn -S | FileCheck %s
+; RUN: opt < %s -passes='require<memoryssa>,gvn' -S | FileCheck --check-prefix=CHECK-MSSA %s
 
 define i32 @main() {
+; CHECK-LABEL: define i32 @main() {
+; CHECK-NEXT:  [[BLOCK1:.*:]]
+; CHECK-NEXT:    ret i32 0
+;
+; CHECK-MSSA-LABEL: define i32 @main() {
+; CHECK-MSSA-NEXT:  [[BLOCK1:.*:]]
+; CHECK-MSSA-NEXT:    ret i32 0
+;
 block1:
-	%z1 = bitcast i32 0 to i32
-	br label %block2
+  %z1 = bitcast i32 0 to i32
+  br label %block2
 block2:
   %z2 = bitcast i32 0 to i32
   ret i32 %z2
 }
 
-; CHECK: define i32 @main() {
-; CHECK-NEXT: block1:
-; CHECK-NEXT:   ret i32 0
-; CHECK-NEXT: }
diff --git a/llvm/test/Transforms/GVN/nonescaping.ll b/llvm/test/Transforms/GVN/nonescaping.ll
index ad6fa1db0f93e..6e978586763f1 100644
--- a/llvm/test/Transforms/GVN/nonescaping.ll
+++ b/llvm/test/Transforms/GVN/nonescaping.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -S -passes=gvn 2>&1 | FileCheck %s
+; RUN: opt < %s -S -passes='require<memoryssa>,gvn' 2>&1 | FileCheck --check-prefix=CHECK-MSSA %s
 
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128"
 
@@ -13,6 +14,11 @@ define i8 @test_malloc(ptr %p) {
 ; CHECK-NEXT:    [[OBJ:%.*]] = call ptr @malloc(i64 16)
 ; CHECK-NEXT:    call void @escape(ptr [[OBJ]])
 ; CHECK-NEXT:    ret i8 0
+;
+; CHECK-MSSA-LABEL: @test_malloc(
+; CHECK-MSSA-NEXT:    [[OBJ:%.*]] = call ptr @malloc(i64 16)
+; CHECK-MSSA-NEXT:    call void @escape(ptr [[OBJ]])
+; CHECK-MSSA-NEXT:    ret i8 0
 ;
   %v1 = load i8, ptr %p
   %obj = call ptr @malloc(i64 16)
@@ -27,6 +33,11 @@ define i8 @test_calloc(ptr %p) {
 ; CHECK-NEXT:    [[OBJ:%.*]] = call ptr @calloc(i64 1, i64 16)
 ; CHECK-NEXT:    call void @escape(ptr [[OBJ]])
 ; CHECK-NEXT:    ret i8 0
+;
+; CHECK-MSSA-LABEL: @test_calloc(
+; CHECK-MSSA-NEXT:    [[OBJ:%.*]] = call ptr @calloc(i64 1, i64 16)
+; CHECK-MSSA-NEXT:    call void @escape(ptr [[OBJ]])
+; CHECK-MSSA-NEXT:    ret i8 0
 ;
   %v1 = load i8, ptr %p
   %obj = call ptr @calloc(i64 1, i64 16)
@@ -41,6 +52,11 @@ define i8 @test_opnew(ptr %p) {
 ; CHECK-NEXT:    [[OBJ:%.*]] = call ptr @_Znwm(i64 16)
 ; CHECK-NEXT:    call void @escape(ptr [[OBJ]])
 ; CHECK-NEXT:    ret i8 0
+;
+; CHECK-MSSA-LABEL: @test_opnew(
+; CHECK-MSSA-NEXT:    [[OBJ:%.*]] = call ptr @_Znwm(i64 16)
+; CHECK-MSSA-NEXT:    call void @escape(ptr [[OBJ]])
+; CHECK-MSSA-NEXT:    ret i8 0
 ;
   %v1 = load i8, ptr %p
   %obj = call ptr @_Znwm(i64 16)
diff --git a/llvm/test/Transforms/GVN/pr14166.ll b/llvm/test/Transforms/GVN/pr14166.ll
index 3379a5f604b0b..0bfd0d8a4a01e 100644
--- a/llvm/test/Transforms/GVN/pr14166.ll
+++ b/llvm/test/Transforms/GVN/pr14166.ll
@@ -1,6 +1,26 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
 ; RUN: opt -disable-basic-aa -passes=gvn -S < %s | FileCheck %s
+; RUN: opt -disable-basic-aa -passes='require<memoryssa>,gvn' -S < %s | FileCheck --check-prefix=CHECK-MSSA %s
 target datalayout = "e-p:32:32:32"
 define <2 x i32> @test1() {
+; CHECK-LABEL: define <2 x i32> @test1() {
+; CHECK-NEXT:    [[V1:%.*]] = alloca <2 x i32>, align 8
+; CHECK-NEXT:    call void @anything(ptr [[V1]])
+; CHECK-NEXT:    [[V2:%.*]] = load <2 x i32>, ptr [[V1]], align 8
+; CHECK-NEXT:    [[V3:%.*]] = inttoptr <2 x i32> [[V2]] to <2 x ptr>
+; CHECK-NEXT:    store <2 x ptr> [[V3]], ptr [[V1]], align 8
+; CHECK-NEXT:    [[TMP1:%.*]] = bitcast <2 x i32> [[V2]] to i64
+; CHECK-NEXT:    ret <2 x i32> [[V2]]
+;
+; CHECK-MSSA-LABEL: define <2 x i32> @test1() {
+; CHECK-MSSA-NEXT:    [[V1:%.*]] = alloca <2 x i32>, align 8
+; CHECK-MSSA-NEXT:    call void @anything(ptr [[V1]])
+; CHECK-MSSA-NEXT:    [[V2:%.*]] = load <2 x i32>, ptr [[V1]], align 8
+; CHECK-MSSA-NEXT:    [[V3:%.*]] = inttoptr <2 x i32> [[V2]] to <2 x ptr>
+; CHECK-MSSA-NEXT:    store <2 x ptr> [[V3]], ptr [[V1]], align 8
+; CHECK-MSSA-NEXT:    [[TMP1:%.*]] = bitcast <2 x i32> [[V2]] to i64
+; CHECK-MSSA-NEXT:    ret <2 x i32> [[V2]]
+;
   %v1 = alloca <2 x i32>
   call void @anything(ptr %v1)
   %v2 = load <2 x i32>, ptr %v1
@@ -8,13 +28,6 @@ define <2 x i32> @test1() {
   store <2 x ptr> %v3, ptr %v1
   %v5 = load <2 x i32>, ptr %v1
   ret <2 x i32> %v5
-; CHECK-LABEL: @test1(
-; CHECK: %v1 = alloca <2 x i32>
-; CHECK: call void @anything(ptr %v1)
-; CHECK: %v2 = load <2 x i32>, ptr %v1
-; CHECK: %v3 = inttoptr <2 x i32> %v2 to <2 x ptr>
-; CHECK: store <2 x ptr> %v3, ptr %v1
-; CHECK: ret <2 x i32> %v2
 }
 
 declare void @anything(ptr)

madhur13490 · 2025-03-07T08:49:24Z

Hi @nikic , I'm changing a few tests in the PR, but if we think MemorySSA is good to test, I will change other tests too. Given that MemorySSA is off by default, I am not sure if we want to ensure they are passing.

madhur13490 · 2025-03-17T04:12:38Z

Gentle ping @nikic @antoniofrighetto

nikic · 2025-03-20T09:39:32Z

llvm/test/Transforms/GVN/assume.ll

 ; RUN: opt < %s -passes=gvn -verify-analysis-invalidation -S | FileCheck %s
+; RUN: opt < %s -passes='require<memoryssa>,gvn' -verify-analysis-invalidation -S | FileCheck --check-prefix=CHECK-MSSA %s


Suggested change

; RUN: opt < %s -passes=gvn -verify-analysis-invalidation -S | FileCheck %s

; RUN: opt < %s -passes='require<memoryssa>,gvn' -verify-analysis-invalidation -S | FileCheck --check-prefix=CHECK-MSSA %s

; RUN: opt < %s -passes=gvn -verify-analysis-invalidation -S | FileCheck %s --check-prefixes=CHECK,MDEP

; RUN: opt < %s -passes='require<memoryssa>,gvn' -verify-analysis-invalidation -S | FileCheck --check-prefixes=CHECK,MSSA %s

Please use check prefixes that look something like this, so that we can see where mdep/mssa differs and where it is the same.

Also this should use -passes='gvn<memoryssa>' instead of require<memoryssa>,gvn. The latter will just preserve MSSA, but not use it.

I get hit by assertion.
GVN.cpp:844: llvm::PreservedAnalyses llvm::GVNPass::run(llvm::Function &, llvm::FunctionAnalysisManager &): Assertion !MemDep && "On-demand computation of MemSSA implies that MemDep is disabled!"' failed.`

Does it require some more setup? Is it expected to work at all?

From the error message, maybe it needs gvn<memoryssa;no-memdep>?

I see. Thanks. That fixed.

nikic · 2025-03-20T11:02:38Z

llvm/test/Transforms/GVN/assume.ll

@@ -8,6 +9,8 @@ define void @assume_true() {
 ; CHECK-LABEL: @assume_true(
 ; CHECK-NEXT:    ret void
 ;
+; CHECK-MSSA-LABEL: @assume_true(
+; CHECK-MSSA-NEXT:    ret void


You have to manually drop the old CHECK-MSSA lines (UTC only handles the ones in check-prefixes).

nikic

This looks reasonable to me now. @antoniofrighetto Any concerns with testing MSSA? Also, do you think it would make sense to make gvn<memoryssa> work by having it imply no-memdep?

madhur13490 · 2025-03-20T14:21:03Z

This looks reasonable to me now. @antoniofrighetto Any concerns with testing MSSA? Also, do you think it would make sense to make gvn<memoryssa> work by having it imply no-memdep?

I second this. Either of them will be in use, so enabling one should imply the disablement of the other.

antoniofrighetto · 2025-03-20T14:27:03Z

This looks reasonable to me now. @antoniofrighetto Any concerns with testing MSSA? Also, do you think it would make sense to make gvn<memoryssa> work by having it imply no-memdep?

I second this. Either of them will be in use, so enabling one should imply the disablement of the other.

That makes sense. I think it would be ideal to do this after flipping the flag.

madhur13490 · 2025-03-20T15:11:05Z

This looks reasonable to me now. @antoniofrighetto Any concerns with testing MSSA? Also, do you think it would make sense to make gvn<memoryssa> work by having it imply no-memdep?

I second this. Either of them will be in use, so enabling one should imply the disablement of the other.

That makes sense. I think it would be ideal to do this after flipping the flag.

Yes, it is ideal but I think we need to ensure what is passing should continue to pass. When we flip the switch, we know what is failing. I will add memoryssa to only those tests which are currently passing. For the failing test cases, I will file Github issue with appropriate keywords.

Does this make sense?

antoniofrighetto

LG

The previous patch in this series is llvm#130261

The previous patch in this series is #130261

The previous patch in this series is llvm#130261

madhur13490 requested a review from nikic March 7, 2025 08:47

llvmbot added the llvm:transforms label Mar 7, 2025

dtcxzyw requested a review from antoniofrighetto March 7, 2025 08:52

nikic reviewed Mar 20, 2025

View reviewed changes

madhur13490 force-pushed the add_mssa_tests branch from 8f00f3f to a9ced58 Compare March 20, 2025 10:48

nikic reviewed Mar 20, 2025

View reviewed changes

[GVN] Add MemorySSA checks in tests 1/N

db9fb63

madhur13490 force-pushed the add_mssa_tests branch from a9ced58 to db9fb63 Compare March 20, 2025 12:01

nikic reviewed Mar 20, 2025

View reviewed changes

antoniofrighetto approved these changes Mar 20, 2025

View reviewed changes

madhur13490 merged commit ad0827d into llvm:main Mar 21, 2025
11 checks passed

madhur13490 added a commit to madhur13490/llvm-project that referenced this pull request Apr 29, 2025

[GVN] Add MSSA checks in tests 2/N

bec1567

The previous patch in this series is llvm#130261

madhur13490 mentioned this pull request Apr 29, 2025

[GVN][NFC] Add MSSA checks in tests 2/N #137814

Merged

madhur13490 added a commit that referenced this pull request May 15, 2025

[GVN][NFC] Add MSSA checks in tests 2/N (#137814)

4077882

The previous patch in this series is #130261

TIFitis pushed a commit to TIFitis/llvm-project that referenced this pull request May 19, 2025

[GVN][NFC] Add MSSA checks in tests 2/N (llvm#137814)

f016aa7

The previous patch in this series is llvm#130261

		; RUN: opt < %s -passes=gvn -verify-analysis-invalidation -S \| FileCheck %s
		; RUN: opt < %s -passes='require<memoryssa>,gvn' -verify-analysis-invalidation -S \| FileCheck --check-prefix=CHECK-MSSA %s

[GVN] Add MemorySSA checks in tests 1/N #130261

[GVN] Add MemorySSA checks in tests 1/N #130261

Uh oh!

Conversation

madhur13490 commented Mar 7, 2025

Uh oh!

llvmbot commented Mar 7, 2025

Uh oh!

madhur13490 commented Mar 7, 2025

Uh oh!

madhur13490 commented Mar 17, 2025

Uh oh!

nikic Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

nikic Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

madhur13490 Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

nikic Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

madhur13490 Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

nikic Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

madhur13490 Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

madhur13490 commented Mar 20, 2025

Uh oh!

antoniofrighetto commented Mar 20, 2025

Uh oh!

madhur13490 commented Mar 20, 2025

Uh oh!

antoniofrighetto left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!