Skip to content

[AArch64][GlobalISel] Do not create LIFETIME instructions in functions. #115669

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
merged 1 commit into from
Nov 11, 2024

Conversation

davemgreen
Copy link
Collaborator

For the same reason that we do not translate lifetime markers in a -O0, we should not translate them for optnone functions too.

For the same reason that we do not translate lifetime markers in a -O0, we
should not translate them for optnone functions too.
@llvmbot
Copy link
Member

llvmbot commented Nov 10, 2024

@llvm/pr-subscribers-backend-aarch64

Author: David Green (davemgreen)

Changes

For the same reason that we do not translate lifetime markers in a -O0, we should not translate them for optnone functions too.


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

2 Files Affected:

  • (modified) llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp (+2-1)
  • (modified) llvm/test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll (+14)
diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index 349c35b3781d16..056f4f41ffca79 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -2166,7 +2166,8 @@ bool IRTranslator::translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID,
   case Intrinsic::lifetime_start:
   case Intrinsic::lifetime_end: {
     // No stack colouring in O0, discard region information.
-    if (MF->getTarget().getOptLevel() == CodeGenOptLevel::None)
+    if (MF->getTarget().getOptLevel() == CodeGenOptLevel::None ||
+        MF->getFunction().hasOptNone())
       return true;
 
     unsigned Op = ID == Intrinsic::lifetime_start ? TargetOpcode::LIFETIME_START
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll b/llvm/test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll
index ed7bcff5160f81..7a67cf3fd4c942 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll
@@ -1477,6 +1477,20 @@ define void @test_lifetime_intrin() {
   ret void
 }
 
+define void @test_lifetime_intrin_optnone() optnone noinline {
+; CHECK-LABEL: name: test_lifetime_intrin_optnone
+; CHECK: RET_ReallyLR
+; O3-LABEL: name: test_lifetime_intrin_optnone
+; O3: {{%[0-9]+}}:_(p0) = G_FRAME_INDEX %stack.0.slot
+; O3-NEXT: G_STORE
+; O3-NEXT: RET_ReallyLR
+  %slot = alloca i8, i32 4
+  call void @llvm.lifetime.start.p0(i64 0, ptr %slot)
+  store volatile i8 10, ptr %slot
+  call void @llvm.lifetime.end.p0(i64 0, ptr %slot)
+  ret void
+}
+
 define void @test_load_store_atomics(ptr %addr) {
 ; CHECK-LABEL: name: test_load_store_atomics
 ; CHECK: [[ADDR:%[0-9]+]]:_(p0) = COPY $x0

@llvmbot
Copy link
Member

llvmbot commented Nov 10, 2024

@llvm/pr-subscribers-llvm-globalisel

Author: David Green (davemgreen)

Changes

For the same reason that we do not translate lifetime markers in a -O0, we should not translate them for optnone functions too.


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

2 Files Affected:

  • (modified) llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp (+2-1)
  • (modified) llvm/test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll (+14)
diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index 349c35b3781d16..056f4f41ffca79 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -2166,7 +2166,8 @@ bool IRTranslator::translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID,
   case Intrinsic::lifetime_start:
   case Intrinsic::lifetime_end: {
     // No stack colouring in O0, discard region information.
-    if (MF->getTarget().getOptLevel() == CodeGenOptLevel::None)
+    if (MF->getTarget().getOptLevel() == CodeGenOptLevel::None ||
+        MF->getFunction().hasOptNone())
       return true;
 
     unsigned Op = ID == Intrinsic::lifetime_start ? TargetOpcode::LIFETIME_START
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll b/llvm/test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll
index ed7bcff5160f81..7a67cf3fd4c942 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll
@@ -1477,6 +1477,20 @@ define void @test_lifetime_intrin() {
   ret void
 }
 
+define void @test_lifetime_intrin_optnone() optnone noinline {
+; CHECK-LABEL: name: test_lifetime_intrin_optnone
+; CHECK: RET_ReallyLR
+; O3-LABEL: name: test_lifetime_intrin_optnone
+; O3: {{%[0-9]+}}:_(p0) = G_FRAME_INDEX %stack.0.slot
+; O3-NEXT: G_STORE
+; O3-NEXT: RET_ReallyLR
+  %slot = alloca i8, i32 4
+  call void @llvm.lifetime.start.p0(i64 0, ptr %slot)
+  store volatile i8 10, ptr %slot
+  call void @llvm.lifetime.end.p0(i64 0, ptr %slot)
+  ret void
+}
+
 define void @test_load_store_atomics(ptr %addr) {
 ; CHECK-LABEL: name: test_load_store_atomics
 ; CHECK: [[ADDR:%[0-9]+]]:_(p0) = COPY $x0

@davemgreen davemgreen merged commit a4e507d into llvm:main Nov 11, 2024
11 checks passed
@davemgreen davemgreen deleted the gh-gi-lifetimeoptnone branch November 11, 2024 09:27
@@ -2166,7 +2166,8 @@ bool IRTranslator::translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID,
case Intrinsic::lifetime_start:
case Intrinsic::lifetime_end: {
// No stack colouring in O0, discard region information.
if (MF->getTarget().getOptLevel() == CodeGenOptLevel::None)
if (MF->getTarget().getOptLevel() == CodeGenOptLevel::None ||
MF->getFunction().hasOptNone())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DAG doesn't check this, fix both together?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like DAG goes via OptLevelChanger, which sets the OptLevel and resets it after handling the function. (And forces into using fast-isel, even if we would not usually use it).

Groverkss pushed a commit to iree-org/llvm-project that referenced this pull request Nov 15, 2024
…s. (llvm#115669)

For the same reason that we do not translate lifetime markers in a -O0,
we should not translate them for optnone functions too.
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.

4 participants