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

[ConstantFold] Fix result type when folding powi.f16 #98681

Merged
merged 4 commits into from
Jul 15, 2024

Conversation

dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Jul 12, 2024

Fixes #98665.

@llvmbot
Copy link
Collaborator

llvmbot commented Jul 12, 2024

@llvm/pr-subscribers-llvm-analysis

Author: Yingwei Zheng (dtcxzyw)

Changes

Fixes #98665.


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

2 Files Affected:

  • (modified) llvm/lib/Analysis/ConstantFolding.cpp (+7-5)
  • (added) llvm/test/Transforms/InstSimplify/ConstProp/pr98665.ll (+11)
diff --git a/llvm/lib/Analysis/ConstantFolding.cpp b/llvm/lib/Analysis/ConstantFolding.cpp
index 962880f68f076..b86f91edeb5b9 100644
--- a/llvm/lib/Analysis/ConstantFolding.cpp
+++ b/llvm/lib/Analysis/ConstantFolding.cpp
@@ -2760,11 +2760,13 @@ static Constant *ConstantFoldIntrinsicCall2(Intrinsic::ID IntrinsicID, Type *Ty,
 
       if (!Ty->isHalfTy() && !Ty->isFloatTy() && !Ty->isDoubleTy())
         return nullptr;
-      if (IntrinsicID == Intrinsic::powi && Ty->isHalfTy())
-        return ConstantFP::get(
-            Ty->getContext(),
-            APFloat((float)std::pow((float)Op1V.convertToDouble(),
-                                    (int)Op2C->getZExtValue())));
+      if (IntrinsicID == Intrinsic::powi && Ty->isHalfTy()) {
+        APFloat Res((float)std::pow((float)Op1V.convertToDouble(),
+                                    (int)Op2C->getZExtValue()));
+        bool unused;
+        Res.convert(APFloat::IEEEhalf(), APFloat::rmNearestTiesToEven, &unused);
+        return ConstantFP::get(Ty->getContext(), Res);
+      }
       if (IntrinsicID == Intrinsic::powi && Ty->isFloatTy())
         return ConstantFP::get(
             Ty->getContext(),
diff --git a/llvm/test/Transforms/InstSimplify/ConstProp/pr98665.ll b/llvm/test/Transforms/InstSimplify/ConstProp/pr98665.ll
new file mode 100644
index 0000000000000..8b7d361840936
--- /dev/null
+++ b/llvm/test/Transforms/InstSimplify/ConstProp/pr98665.ll
@@ -0,0 +1,11 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt < %s -passes=instsimplify -S | FileCheck %s
+; Make sure that the type is correct after constant folding
+
+define half @pr98665() {
+; CHECK-LABEL: define half @pr98665() {
+; CHECK-NEXT:    ret half 0xH3C00
+;
+  %x = call half @llvm.powi.f16.i32(half 0xH3C00, i32 1)
+  ret half %x
+}

@llvmbot
Copy link
Collaborator

llvmbot commented Jul 12, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

Fixes #98665.


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

2 Files Affected:

  • (modified) llvm/lib/Analysis/ConstantFolding.cpp (+7-5)
  • (added) llvm/test/Transforms/InstSimplify/ConstProp/pr98665.ll (+11)
diff --git a/llvm/lib/Analysis/ConstantFolding.cpp b/llvm/lib/Analysis/ConstantFolding.cpp
index 962880f68f076..b86f91edeb5b9 100644
--- a/llvm/lib/Analysis/ConstantFolding.cpp
+++ b/llvm/lib/Analysis/ConstantFolding.cpp
@@ -2760,11 +2760,13 @@ static Constant *ConstantFoldIntrinsicCall2(Intrinsic::ID IntrinsicID, Type *Ty,
 
       if (!Ty->isHalfTy() && !Ty->isFloatTy() && !Ty->isDoubleTy())
         return nullptr;
-      if (IntrinsicID == Intrinsic::powi && Ty->isHalfTy())
-        return ConstantFP::get(
-            Ty->getContext(),
-            APFloat((float)std::pow((float)Op1V.convertToDouble(),
-                                    (int)Op2C->getZExtValue())));
+      if (IntrinsicID == Intrinsic::powi && Ty->isHalfTy()) {
+        APFloat Res((float)std::pow((float)Op1V.convertToDouble(),
+                                    (int)Op2C->getZExtValue()));
+        bool unused;
+        Res.convert(APFloat::IEEEhalf(), APFloat::rmNearestTiesToEven, &unused);
+        return ConstantFP::get(Ty->getContext(), Res);
+      }
       if (IntrinsicID == Intrinsic::powi && Ty->isFloatTy())
         return ConstantFP::get(
             Ty->getContext(),
diff --git a/llvm/test/Transforms/InstSimplify/ConstProp/pr98665.ll b/llvm/test/Transforms/InstSimplify/ConstProp/pr98665.ll
new file mode 100644
index 0000000000000..8b7d361840936
--- /dev/null
+++ b/llvm/test/Transforms/InstSimplify/ConstProp/pr98665.ll
@@ -0,0 +1,11 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt < %s -passes=instsimplify -S | FileCheck %s
+; Make sure that the type is correct after constant folding
+
+define half @pr98665() {
+; CHECK-LABEL: define half @pr98665() {
+; CHECK-NEXT:    ret half 0xH3C00
+;
+  %x = call half @llvm.powi.f16.i32(half 0xH3C00, i32 1)
+  ret half %x
+}

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

LGTM, though this code could really do with some cleanup. Why are there three different ifs checking for IntrinsicID == Intrinsic::powi when there is a perfectly good switch just before it?

@dtcxzyw dtcxzyw merged commit 34bfed6 into llvm:main Jul 15, 2024
7 checks passed
@dtcxzyw dtcxzyw deleted the fix-pr98665 branch July 15, 2024 05:00
@dtcxzyw
Copy link
Member Author

dtcxzyw commented Jul 15, 2024

dtcxzyw/llvm-opt-benchmark#961
I will post a fix.

@llvm-ci
Copy link
Collaborator

llvm-ci commented Jul 15, 2024

LLVM Buildbot has detected a new failure on builder sanitizer-aarch64-linux running on sanitizer-buildbot7 while building llvm at step 2 "annotate".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/51/builds/1315

Here is the relevant piece of the build log for the reference:

Step 2 (annotate) failure: 'python ../sanitizer_buildbot/sanitizers/zorg/buildbot/builders/sanitizers/buildbot_selector.py' (failure)
...
llvm-lit: /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/fuzzer/lit.cfg.py:60: note: linux feature available
llvm-lit: /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/fuzzer/lit.cfg.py:36: note: lsan feature available
llvm-lit: /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/fuzzer/lit.cfg.py:48: note: msan feature available
llvm-lit: /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/fuzzer/lit.cfg.py:60: note: linux feature available
llvm-lit: /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/fuzzer/lit.cfg.py:36: note: lsan feature available
llvm-lit: /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/fuzzer/lit.cfg.py:48: note: msan feature available
llvm-lit: /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/fuzzer/lit.cfg.py:60: note: linux feature available
llvm-lit: /b/sanitizer-aarch64-linux/build/llvm-project/llvm/utils/lit/lit/main.py:72: note: The test suite configuration requested an individual test timeout of 0 seconds but a timeout of 900 seconds was requested on the command line. Forcing timeout to be 900 seconds.
-- Testing: 5483 tests, 48 workers --
Testing:  0.. 10.. 20.. 30
FAIL: LeakSanitizer-AddressSanitizer-aarch64 :: TestCases/use_registers.cpp (1768 of 5483)
******************** TEST 'LeakSanitizer-AddressSanitizer-aarch64 :: TestCases/use_registers.cpp' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
Test alloc: 0x51a000000080 

=================================================================
==3246175==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 1337 byte(s) in 1 object(s) allocated from:
    #0 0xb44217adb6f8 in malloc /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:68:3
    #1 0xb44217c04cfc in registers_thread_func /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/lsan/TestCases/use_registers.cpp:16:13
    #2 0xb44217ad8f64 in asan_thread_start(void*) /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/lib/asan/asan_interceptors.cpp:239:28
    #3 0xfe0232c3ba48  (/lib/aarch64-linux-gnu/libc.so.6+0xeba48) (BuildId: 3119d7bcf68a34e21742cd08095272de601d373f)

Objects leaked above:
0x51a000000080 (1337 bytes)

SUMMARY: AddressSanitizer: 1337 byte(s) leaked in 1 allocation(s).

--
Command Output (stderr):
--
RUN: at line 2: /b/sanitizer-aarch64-linux/build/build_default/./bin/clang  --driver-mode=g++ -O0   -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta   -gline-tables-only -fsanitize=address -I/b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/lsan/../ -pthread /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/lsan/TestCases/use_registers.cpp -o /b/sanitizer-aarch64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/lsan/AARCH64AsanConfig/TestCases/Output/use_registers.cpp.tmp
+ /b/sanitizer-aarch64-linux/build/build_default/./bin/clang --driver-mode=g++ -O0 -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta -gline-tables-only -fsanitize=address -I/b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/lsan/../ -pthread /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/lsan/TestCases/use_registers.cpp -o /b/sanitizer-aarch64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/lsan/AARCH64AsanConfig/TestCases/Output/use_registers.cpp.tmp
RUN: at line 3: env LSAN_OPTIONS=:detect_leaks=1:"report_objects=1:use_stacks=0:use_registers=0" not  /b/sanitizer-aarch64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/lsan/AARCH64AsanConfig/TestCases/Output/use_registers.cpp.tmp 2>&1 | FileCheck /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/lsan/TestCases/use_registers.cpp
+ env LSAN_OPTIONS=:detect_leaks=1:report_objects=1:use_stacks=0:use_registers=0 not /b/sanitizer-aarch64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/lsan/AARCH64AsanConfig/TestCases/Output/use_registers.cpp.tmp
+ FileCheck /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/lsan/TestCases/use_registers.cpp
RUN: at line 4: env LSAN_OPTIONS=:detect_leaks=1:"report_objects=1:use_stacks=0:use_registers=1"  /b/sanitizer-aarch64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/lsan/AARCH64AsanConfig/TestCases/Output/use_registers.cpp.tmp 2>&1
+ env LSAN_OPTIONS=:detect_leaks=1:report_objects=1:use_stacks=0:use_registers=1 /b/sanitizer-aarch64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/lsan/AARCH64AsanConfig/TestCases/Output/use_registers.cpp.tmp

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 

2 warning(s) in tests
Step 9 (test compiler-rt symbolizer) failure: test compiler-rt symbolizer (failure)
...
llvm-lit: /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/fuzzer/lit.cfg.py:60: note: linux feature available
llvm-lit: /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/fuzzer/lit.cfg.py:36: note: lsan feature available
llvm-lit: /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/fuzzer/lit.cfg.py:48: note: msan feature available
llvm-lit: /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/fuzzer/lit.cfg.py:60: note: linux feature available
llvm-lit: /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/fuzzer/lit.cfg.py:36: note: lsan feature available
llvm-lit: /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/fuzzer/lit.cfg.py:48: note: msan feature available
llvm-lit: /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/fuzzer/lit.cfg.py:60: note: linux feature available
llvm-lit: /b/sanitizer-aarch64-linux/build/llvm-project/llvm/utils/lit/lit/main.py:72: note: The test suite configuration requested an individual test timeout of 0 seconds but a timeout of 900 seconds was requested on the command line. Forcing timeout to be 900 seconds.
-- Testing: 5483 tests, 48 workers --
Testing:  0.. 10.. 20.. 30
FAIL: LeakSanitizer-AddressSanitizer-aarch64 :: TestCases/use_registers.cpp (1768 of 5483)
******************** TEST 'LeakSanitizer-AddressSanitizer-aarch64 :: TestCases/use_registers.cpp' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
Test alloc: 0x51a000000080 

=================================================================
==3246175==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 1337 byte(s) in 1 object(s) allocated from:
    #0 0xb44217adb6f8 in malloc /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:68:3
    #1 0xb44217c04cfc in registers_thread_func /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/lsan/TestCases/use_registers.cpp:16:13
    #2 0xb44217ad8f64 in asan_thread_start(void*) /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/lib/asan/asan_interceptors.cpp:239:28
    #3 0xfe0232c3ba48  (/lib/aarch64-linux-gnu/libc.so.6+0xeba48) (BuildId: 3119d7bcf68a34e21742cd08095272de601d373f)

Objects leaked above:
0x51a000000080 (1337 bytes)

SUMMARY: AddressSanitizer: 1337 byte(s) leaked in 1 allocation(s).

--
Command Output (stderr):
--
RUN: at line 2: /b/sanitizer-aarch64-linux/build/build_default/./bin/clang  --driver-mode=g++ -O0   -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta   -gline-tables-only -fsanitize=address -I/b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/lsan/../ -pthread /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/lsan/TestCases/use_registers.cpp -o /b/sanitizer-aarch64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/lsan/AARCH64AsanConfig/TestCases/Output/use_registers.cpp.tmp
+ /b/sanitizer-aarch64-linux/build/build_default/./bin/clang --driver-mode=g++ -O0 -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta -gline-tables-only -fsanitize=address -I/b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/lsan/../ -pthread /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/lsan/TestCases/use_registers.cpp -o /b/sanitizer-aarch64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/lsan/AARCH64AsanConfig/TestCases/Output/use_registers.cpp.tmp
RUN: at line 3: env LSAN_OPTIONS=:detect_leaks=1:"report_objects=1:use_stacks=0:use_registers=0" not  /b/sanitizer-aarch64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/lsan/AARCH64AsanConfig/TestCases/Output/use_registers.cpp.tmp 2>&1 | FileCheck /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/lsan/TestCases/use_registers.cpp
+ env LSAN_OPTIONS=:detect_leaks=1:report_objects=1:use_stacks=0:use_registers=0 not /b/sanitizer-aarch64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/lsan/AARCH64AsanConfig/TestCases/Output/use_registers.cpp.tmp
+ FileCheck /b/sanitizer-aarch64-linux/build/llvm-project/compiler-rt/test/lsan/TestCases/use_registers.cpp
RUN: at line 4: env LSAN_OPTIONS=:detect_leaks=1:"report_objects=1:use_stacks=0:use_registers=1"  /b/sanitizer-aarch64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/lsan/AARCH64AsanConfig/TestCases/Output/use_registers.cpp.tmp 2>&1
+ env LSAN_OPTIONS=:detect_leaks=1:report_objects=1:use_stacks=0:use_registers=1 /b/sanitizer-aarch64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/lsan/AARCH64AsanConfig/TestCases/Output/use_registers.cpp.tmp

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..

2 warning(s) in tests

@dtcxzyw
Copy link
Member Author

dtcxzyw commented Jul 15, 2024

dtcxzyw/llvm-opt-benchmark#961 I will post a fix.

See #98860.

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.

Misoptimization: EarlyCSEPass uses replaces powi.f16 with float result
4 participants