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

[WASM] Fix for wasi libc build break add tan to RuntimeLibcallSignatureTable #95082

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

farzonl
Copy link
Member

@farzonl farzonl commented Jun 11, 2024

The wasm backend fetches the tan runtime lib call in llvm/include/llvm/IR/RuntimeLibcalls.def via StaticLibcallNameMap(), but ignores the runtime function because a function sinature mapping is not specified in RuntimeLibcallSignatureTable(). The fix is to specify the function signatures for float32-128.

This is a fix for a build break reported on PR #94559 (comment).

@llvmbot
Copy link
Collaborator

llvmbot commented Jun 11, 2024

@llvm/pr-subscribers-backend-webassembly

Author: Farzon Lotfi (farzonl)

Changes

The wasm backend fetches the tan runtime lib call in llvm/include/llvm/IR/RuntimeLibcalls.def via StaticLibcallNameMap(), but ignores the runtime function because a function sinature mapping is not specified in RuntimeLibcallSignatureTable(). The fix is to specify the function signatures for float32-128.

This is a fix for a build break reported on PR #94559 (comment).


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

2 Files Affected:

  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp (+3)
  • (modified) llvm/test/CodeGen/WebAssembly/libcalls.ll (+30-27)
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp
index d9936557776ba..20e50c8c9e1ae 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp
@@ -201,6 +201,9 @@ struct RuntimeLibcallSignatureTable {
     Table[RTLIB::COS_F32] = f32_func_f32;
     Table[RTLIB::COS_F64] = f64_func_f64;
     Table[RTLIB::COS_F128] = i64_i64_func_i64_i64;
+    Table[RTLIB::TAN_F32] = f32_func_f32;
+    Table[RTLIB::TAN_F64] = f64_func_f64;
+    Table[RTLIB::TAN_F128] = i64_i64_func_i64_i64;
     Table[RTLIB::SINCOS_F32] = func_f32_iPTR_iPTR;
     Table[RTLIB::SINCOS_F64] = func_f64_iPTR_iPTR;
     Table[RTLIB::SINCOS_F128] = func_i64_i64_iPTR_iPTR;
diff --git a/llvm/test/CodeGen/WebAssembly/libcalls.ll b/llvm/test/CodeGen/WebAssembly/libcalls.ll
index 4f57c347a1a33..d465fd431d0f4 100644
--- a/llvm/test/CodeGen/WebAssembly/libcalls.ll
+++ b/llvm/test/CodeGen/WebAssembly/libcalls.ll
@@ -12,6 +12,7 @@ declare fp128 @llvm.nearbyint.f128(fp128)
 declare fp128 @llvm.pow.f128(fp128, fp128)
 declare fp128 @llvm.powi.f128.i32(fp128, i32)
 
+declare double @llvm.tan.f64(double)
 declare double @llvm.cos.f64(double)
 declare double @llvm.log10.f64(double)
 declare double @llvm.pow.f64(double, double)
@@ -240,39 +241,40 @@ define double @f64libcalls(double %x, double %y, i32 %z) {
 ; CHECK:         .functype f64libcalls (f64, f64, i32) -> (f64)
 ; CHECK-NEXT:    .local i32
 ; CHECK-NEXT:  # %bb.0:
-; CHECK-NEXT:    global.get $push11=, __stack_pointer
-; CHECK-NEXT:    i32.const $push12=, 16
-; CHECK-NEXT:    i32.sub $push18=, $pop11, $pop12
-; CHECK-NEXT:    local.tee $push17=, 3, $pop18
-; CHECK-NEXT:    global.set __stack_pointer, $pop17
-; CHECK-NEXT:    local.get $push22=, 0
-; CHECK-NEXT:    local.get $push19=, 0
-; CHECK-NEXT:    call $push0=, cos, $pop19
+; CHECK-NEXT:    global.get $push12=, __stack_pointer
+; CHECK-NEXT:    i32.const $push13=, 16
+; CHECK-NEXT:    i32.sub  $push19=, $pop12, $pop13
+; CHECK-NEXT:    local.tee $push18=, 3, $pop19
+; CHECK-NEXT:    global.set __stack_pointer, $pop18
+; CHECK-NEXT:    local.get $push23=, 0
+; CHECK-NEXT:    local.get $push20=, 0
+; CHECK-NEXT:    call $push0=, cos, $pop20
 ; CHECK-NEXT:    call $push1=, log10, $pop0
-; CHECK-NEXT:    local.get $push20=, 1
-; CHECK-NEXT:    call $push2=, pow, $pop1, $pop20
-; CHECK-NEXT:    local.get $push21=, 2
-; CHECK-NEXT:    call $push3=, __powidf2, $pop2, $pop21
+; CHECK-NEXT:    local.get $push21=, 1
+; CHECK-NEXT:    call $push2=, pow, $pop1, $pop21
+; CHECK-NEXT:    local.get $push22=, 2
+; CHECK-NEXT:    call $push3=, __powidf2, $pop2, $pop22
 ; CHECK-NEXT:    call $push4=, log, $pop3
 ; CHECK-NEXT:    call $push5=, exp, $pop4
 ; CHECK-NEXT:    call $push6=, exp10, $pop5
 ; CHECK-NEXT:    call $push7=, cbrt, $pop6
 ; CHECK-NEXT:    call $push8=, lround, $pop7
-; CHECK-NEXT:    call $push9=, ldexp, $pop22, $pop8
-; CHECK-NEXT:    local.get $push23=, 3
-; CHECK-NEXT:    i32.const $push15=, 12
-; CHECK-NEXT:    i32.add $push16=, $pop23, $pop15
-; CHECK-NEXT:    call $push24=, frexp, $pop9, $pop16
-; CHECK-NEXT:    local.set 0, $pop24
-; CHECK-NEXT:    local.get $push25=, 3
-; CHECK-NEXT:    i32.load $push10=, 12($pop25)
-; CHECK-NEXT:    call escape_value, $pop10
+; CHECK-NEXT:    call $push9=, ldexp, $pop23, $pop8
+; CHECK-NEXT:    call $push10=, tan, $pop9
+; CHECK-NEXT:    local.get $push24=, 3
+; CHECK-NEXT:    i32.const $push16=, 12
+; CHECK-NEXT:    i32.add  $push17=, $pop24, $pop16
+; CHECK-NEXT:    call $push25=, frexp, $pop10, $pop17
+; CHECK-NEXT:    local.set 0, $pop25
 ; CHECK-NEXT:    local.get $push26=, 3
-; CHECK-NEXT:    i32.const $push13=, 16
-; CHECK-NEXT:    i32.add $push14=, $pop26, $pop13
-; CHECK-NEXT:    global.set __stack_pointer, $pop14
-; CHECK-NEXT:    local.get $push27=, 0
-; CHECK-NEXT:    return $pop27
+; CHECK-NEXT:    i32.load $push11=, 12($pop26)
+; CHECK-NEXT:    call escape_value, $pop11
+; CHECK-NEXT:    local.get $push27=, 3
+; CHECK-NEXT:    i32.const $push14=, 16
+; CHECK-NEXT:    i32.add  $push15=, $pop27, $pop14
+; CHECK-NEXT:    global.set __stack_pointer, $pop15
+; CHECK-NEXT:    local.get $push28=, 0
+; CHECK-NEXT:    return $pop28
 
 
  %a = call double @llvm.cos.f64(double %x)
@@ -285,7 +287,8 @@ define double @f64libcalls(double %x, double %y, i32 %z) {
  %h = call fast double @llvm.pow.f64(double %g, double 0x3FD5555555555555)
  %i = call i32 @llvm.lround(double %h)
  %j = call double @llvm.ldexp.f64.i32(double %x, i32 %i);
- %result = call {double, i32} @llvm.frexp.f64.i32(double %j)
+ %k = call double @llvm.tan.f64(double %j)
+ %result = call {double, i32} @llvm.frexp.f64.i32(double %k)
  %result.0 = extractvalue { double, i32 } %result, 0
  %result.1 = extractvalue { double, i32 } %result, 1
  call void @escape_value(i32 %result.1)

@farzonl
Copy link
Member Author

farzonl commented Jun 11, 2024

@glandium this ended up being a simple fix. FYI to @dschuff and @HerrCai0907. Want to make sure this is ok to merge.

@farzonl farzonl changed the title [WASM] Fix for wasi libc build break [WASM] Fix for wasi libc build break add tan to RuntimeLibcallSignatureTable Jun 11, 2024
The wasm backend fetches the tan runtime lib call in
`llvm/include/llvm/IR/RuntimeLibcalls.def` via `StaticLibcallNameMap()`,
but ignores the runtime function because a function sinature mapping is not specified
in RuntimeLibcallSignatureTable(). The fix is to specify the function signatures for float32-128.
@farzonl farzonl merged commit 38ccee0 into llvm:main Jun 11, 2024
4 of 6 checks passed
@sbc100
Copy link
Collaborator

sbc100 commented Jun 11, 2024

Thanks for the fix! This also fixed the emscripten roller which was temporarily broken.

Just FIY, we normally use [WebAssembly] in PR titles for the Wasm backend. (And just to be very nitpicky we try to never write Wasm in all caps)

@HerrCai0907 HerrCai0907 mentioned this pull request Jun 13, 2024
@farzonl farzonl deleted the fix-for-wasm-build-break branch July 14, 2024 08:51
farzonl added a commit that referenced this pull request Jul 19, 2024
…#98755)

## Change:
- WebAssemblyRuntimeLibcallSignatures.cpp: Expose the RTLIB's for use by
WASM
-  Add trig specific test cases

## History
This change is part of an implementation of
#87367 investigation on
supporting IEEE math operations as intrinsics.
Which was discussed in this RFC:
https://discourse.llvm.org/t/rfc-all-the-math-intrinsics/78294

This change adds wasm lowering cases for `acos`, `asin`, `atan`, `cosh`,
`sinh`, and `tanh`.

#70079
#70080
#70081
#70083
#70084
#95966

## Why Web Assembly?
From past changes to try and support constraint intrinsics the changes
to the trig builtins to emit intrinsics\constraint intrinsics broke the
WASM build. This is an attempt to preempt any such build break.

- #95082
-
#94559 (comment)
sgundapa pushed a commit to sgundapa/upstream_effort that referenced this pull request Jul 23, 2024
…llvm#98755)

## Change:
- WebAssemblyRuntimeLibcallSignatures.cpp: Expose the RTLIB's for use by
WASM
-  Add trig specific test cases

## History
This change is part of an implementation of
llvm#87367 investigation on
supporting IEEE math operations as intrinsics.
Which was discussed in this RFC:
https://discourse.llvm.org/t/rfc-all-the-math-intrinsics/78294

This change adds wasm lowering cases for `acos`, `asin`, `atan`, `cosh`,
`sinh`, and `tanh`.

llvm#70079
llvm#70080
llvm#70081
llvm#70083
llvm#70084
llvm#95966

## Why Web Assembly?
From past changes to try and support constraint intrinsics the changes
to the trig builtins to emit intrinsics\constraint intrinsics broke the
WASM build. This is an attempt to preempt any such build break.

- llvm#95082
-
llvm#94559 (comment)
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
…#98755)

## Change:
- WebAssemblyRuntimeLibcallSignatures.cpp: Expose the RTLIB's for use by
WASM
-  Add trig specific test cases

## History
This change is part of an implementation of
#87367 investigation on
supporting IEEE math operations as intrinsics.
Which was discussed in this RFC:
https://discourse.llvm.org/t/rfc-all-the-math-intrinsics/78294

This change adds wasm lowering cases for `acos`, `asin`, `atan`, `cosh`,
`sinh`, and `tanh`.

#70079
#70080
#70081
#70083
#70084
#95966

## Why Web Assembly?
From past changes to try and support constraint intrinsics the changes
to the trig builtins to emit intrinsics\constraint intrinsics broke the
WASM build. This is an attempt to preempt any such build break.

- #95082
-
#94559 (comment)
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.

5 participants