Skip to content

[NVPTX][NFC] Remove unneeded declarations in test #101167

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
Aug 1, 2024

Conversation

hdelan
Copy link

@hdelan hdelan commented Jul 30, 2024

Only the bf16 declarations are needed, as only they are lowered in AutoUpgrade.cpp.
f16 and other builtins have LLVM intrinsics already defined.

Only the bf16 declarations are needed, as only they are lowered in
AutoUpgrade.cpp.
@llvmbot
Copy link
Member

llvmbot commented Jul 31, 2024

@llvm/pr-subscribers-backend-nvptx

Author: Hugh Delaney (hdelan)

Changes

Only the bf16 declarations are needed, as only they are lowered in AutoUpgrade.cpp.
f16 and other builtins have LLVM intrinsics already defined.


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

1 Files Affected:

  • (modified) llvm/test/CodeGen/NVPTX/math-intrins-sm80-ptx70-autoupgrade.ll (-24)
diff --git a/llvm/test/CodeGen/NVPTX/math-intrins-sm80-ptx70-autoupgrade.ll b/llvm/test/CodeGen/NVPTX/math-intrins-sm80-ptx70-autoupgrade.ll
index 34b9c08509326..3e6477c199577 100644
--- a/llvm/test/CodeGen/NVPTX/math-intrins-sm80-ptx70-autoupgrade.ll
+++ b/llvm/test/CodeGen/NVPTX/math-intrins-sm80-ptx70-autoupgrade.ll
@@ -6,40 +6,16 @@ declare i32 @llvm.nvvm.abs.bf16x2(i32)
 declare i16 @llvm.nvvm.neg.bf16(i16)
 declare i32 @llvm.nvvm.neg.bf16x2(i32)
 
-declare float @llvm.nvvm.fmin.nan.f(float, float)
-declare float @llvm.nvvm.fmin.ftz.nan.f(float, float)
-declare half @llvm.nvvm.fmin.f16(half, half)
-declare half @llvm.nvvm.fmin.ftz.f16(half, half)
-declare half @llvm.nvvm.fmin.nan.f16(half, half)
-declare half @llvm.nvvm.fmin.ftz.nan.f16(half, half)
-declare <2 x half> @llvm.nvvm.fmin.f16x2(<2 x half>, <2 x half>)
-declare <2 x half> @llvm.nvvm.fmin.ftz.f16x2(<2 x half>, <2 x half>)
-declare <2 x half> @llvm.nvvm.fmin.nan.f16x2(<2 x half>, <2 x half>)
-declare <2 x half> @llvm.nvvm.fmin.ftz.nan.f16x2(<2 x half>, <2 x half>)
 declare i16 @llvm.nvvm.fmin.bf16(i16, i16)
 declare i16 @llvm.nvvm.fmin.nan.bf16(i16, i16)
 declare i32 @llvm.nvvm.fmin.bf16x2(i32, i32)
 declare i32 @llvm.nvvm.fmin.nan.bf16x2(i32, i32)
 
-declare float @llvm.nvvm.fmax.nan.f(float, float)
-declare float @llvm.nvvm.fmax.ftz.nan.f(float, float)
-declare half @llvm.nvvm.fmax.f16(half, half)
-declare half @llvm.nvvm.fmax.ftz.f16(half, half)
-declare half @llvm.nvvm.fmax.nan.f16(half, half)
-declare half @llvm.nvvm.fmax.ftz.nan.f16(half, half)
-declare <2 x half> @llvm.nvvm.fmax.f16x2(<2 x half>, <2 x half>)
-declare <2 x half> @llvm.nvvm.fmax.ftz.f16x2(<2 x half>, <2 x half>)
-declare <2 x half> @llvm.nvvm.fmax.nan.f16x2(<2 x half>, <2 x half>)
-declare <2 x half> @llvm.nvvm.fmax.ftz.nan.f16x2(<2 x half>, <2 x half>)
 declare i16 @llvm.nvvm.fmax.bf16(i16, i16)
 declare i16 @llvm.nvvm.fmax.nan.bf16(i16, i16)
 declare i32 @llvm.nvvm.fmax.bf16x2(i32, i32)
 declare i32 @llvm.nvvm.fmax.nan.bf16x2(i32, i32)
 
-declare half @llvm.nvvm.fma.rn.relu.f16(half, half, half)
-declare half @llvm.nvvm.fma.rn.ftz.relu.f16(half, half, half)
-declare <2 x half> @llvm.nvvm.fma.rn.relu.f16x2(<2 x half>, <2 x half>, <2 x half>)
-declare <2 x half> @llvm.nvvm.fma.rn.ftz.relu.f16x2(<2 x half>, <2 x half>, <2 x half>)
 declare i16 @llvm.nvvm.fma.rn.bf16(i16, i16, i16)
 declare i16 @llvm.nvvm.fma.rn.relu.bf16(i16, i16, i16)
 declare i32 @llvm.nvvm.fma.rn.bf16x2(i32, i32, i32)

@hdelan
Copy link
Author

hdelan commented Aug 1, 2024

Ping @Artem-B @jholewinski

@gonzalobg
Copy link
Contributor

Are these declarations tested elsewhere?

It's not clear to me that the fix is removing them, vs adding missing tests for them.

@hdelan
Copy link
Author

hdelan commented Aug 1, 2024

Are these declarations tested elsewhere?

It's not clear to me that the fix is removing them, vs adding missing tests for them.

These declarations that I have removed are used and their PTX lowering tested in this file. But they don't need to be declared in the test file since they are LLVM intrinsics. By contrast the bf16 llvm. declarations are needed since the bf16 funcs are not real LLVM intrinsics. Their lowerings are hoisted here

@ldrumm ldrumm merged commit 3d1e1d9 into llvm:main Aug 1, 2024
8 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.

4 participants