Skip to content
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

[IR] Fix crashes caused by #85592 #87169

Merged
merged 1 commit into from
Apr 2, 2024
Merged

[IR] Fix crashes caused by #85592 #87169

merged 1 commit into from
Apr 2, 2024

Conversation

elhewaty
Copy link
Contributor

@elhewaty elhewaty commented Mar 30, 2024

This patch fixes the crash caused by the pull request:
#85592

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 30, 2024

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-llvm-transforms

Author: None (elhewaty)

Changes

This patch fixes the crash caused by pull request: https://github.com/llvm/llvm-project/pull/85592


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

2 Files Affected:

  • (modified) llvm/lib/IR/Operator.cpp (+3-2)
  • (modified) llvm/test/Transforms/FunctionAttrs/noundef.ll (+14)
diff --git a/llvm/lib/IR/Operator.cpp b/llvm/lib/IR/Operator.cpp
index 495769279e3363..7b4449cd825f9b 100644
--- a/llvm/lib/IR/Operator.cpp
+++ b/llvm/lib/IR/Operator.cpp
@@ -28,8 +28,9 @@ bool Operator::hasPoisonGeneratingFlags() const {
     return OBO->hasNoUnsignedWrap() || OBO->hasNoSignedWrap();
   }
   case Instruction::Trunc: {
-    auto *TI = dyn_cast<TruncInst>(this);
-    return TI->hasNoUnsignedWrap() || TI->hasNoSignedWrap();
+    if (auto *TI = dyn_cast<TruncInst>(this))
+      return TI->hasNoUnsignedWrap() || TI->hasNoSignedWrap();
+    return false;
   }
   case Instruction::UDiv:
   case Instruction::SDiv:
diff --git a/llvm/test/Transforms/FunctionAttrs/noundef.ll b/llvm/test/Transforms/FunctionAttrs/noundef.ll
index 946b562f39553e..9ab37082a30329 100644
--- a/llvm/test/Transforms/FunctionAttrs/noundef.ll
+++ b/llvm/test/Transforms/FunctionAttrs/noundef.ll
@@ -1,6 +1,8 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
 ; RUN: opt < %s -passes='function-attrs' -S | FileCheck %s
 
+@g_var = external global [0 x i8]
+
 define i32 @test_ret_constant() {
 ; CHECK-LABEL: define noundef i32 @test_ret_constant(
 ; CHECK-SAME: ) #[[ATTR0:[0-9]+]] {
@@ -152,3 +154,15 @@ define i32 @test_ret_constant_msan() sanitize_memory {
 ;
   ret i32 0
 }
+
+define i64 @test_trunc_with_constexpr() {
+; CHECK-LABEL: define noundef i64 @test_trunc_with_constexpr(
+; CHECK-SAME: ) #[[ATTR0]] {
+; CHECK-NEXT:    [[ADD:%.*]] = add i32 trunc (i64 sub (i64 0, i64 ptrtoint (ptr @g_var to i64)) to i32), 1
+; CHECK-NEXT:    [[CONV:%.*]] = sext i32 [[ADD]] to i64
+; CHECK-NEXT:    ret i64 [[CONV]]
+;
+  %add = add i32 trunc (i64 sub (i64 0, i64 ptrtoint (ptr @g_var to i64)) to i32), 1
+  %conv = sext i32 %add to i64
+  ret i64 %conv
+}

@elhewaty
Copy link
Contributor Author

@dtcxzyw please review

Copy link
Contributor

@shiltian shiltian left a comment

Choose a reason for hiding this comment

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

LG

@@ -28,8 +28,9 @@ bool Operator::hasPoisonGeneratingFlags() const {
return OBO->hasNoUnsignedWrap() || OBO->hasNoSignedWrap();
}
case Instruction::Trunc: {
auto *TI = dyn_cast<TruncInst>(this);
Copy link
Contributor

@shiltian shiltian Mar 30, 2024

Choose a reason for hiding this comment

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

This style is really bad. Either use a cast directly w/o a check or check the result before use.

Copy link
Contributor

@ronlieb ronlieb left a comment

Choose a reason for hiding this comment

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

i applied your fix to our downstream builds that saw a similar issue ,and it resolved.
thanks will let someone else approve.

searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Mar 31, 2024
pullls in experimentally to try and fix asan hip build
[IR] Fix crashes caused by llvm#85592 llvm#87169

Change-Id: Iee30a92a0e28b6949b69c34047fe2df9aabdd424
Copy link
Member

@dtcxzyw dtcxzyw left a comment

Choose a reason for hiding this comment

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

LGTM.

@elhewaty
Copy link
Contributor Author

elhewaty commented Apr 1, 2024

@dtcxzyw ping, I think this is ready to be merged, you and @shiltian approved.

@dtcxzyw dtcxzyw linked an issue Apr 2, 2024 that may be closed by this pull request
@dtcxzyw dtcxzyw merged commit fa8dc36 into llvm:main Apr 2, 2024
6 of 7 checks passed
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.

Crash in Loop Strength Reduction pass when trying to PGO zsh
5 participants