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

[clang] Add tanf16 builtin and support for tan constrained intrinsic #93314

Merged
merged 3 commits into from
May 29, 2024

Conversation

farzonl
Copy link
Member

@farzonl farzonl commented May 24, 2024

In LLVM, the llvm.experimental.constrained.cos and llvm.experimental.constrained.sin intrinsics are used for performing cosine and sine calculations with additional constraints on floating-point operations. This behavior is expected for all floating-point math intrinsics. This change adds these constraints for the tan intrinsic.

  • Builtins.td - replace TanF128 with F16F128MathTemplate
  • CGBuiltin.cpp - map existing tan builtins to tan and constrained_tan intrinsic
  • ConstrainedOps.def map tan and constrained_tan to an ISDOpcode.
  • ISDOpcodes.h - define tan and strict tan opcodes

resolves #91421

@farzonl farzonl self-assigned this May 24, 2024
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:X86 clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen llvm:SelectionDAG SelectionDAGISel as well llvm:ir labels May 24, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented May 24, 2024

@llvm/pr-subscribers-llvm-ir
@llvm/pr-subscribers-llvm-selectiondag
@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: Farzon Lotfi (farzonl)

Changes
  • Builtins.td - replace TanF128 with F16F128MathTemplate
  • CGBuiltin.cpp - map existing tan builtins to tan and constrained_tan intrinsic
  • ConstrainedOps.def map tan and constrained_tan to an ISDOpcode.
  • ISDOpcodes.h - define tan and strict tan opcodes

resolves #91421


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

12 Files Affected:

  • (modified) clang/include/clang/Basic/Builtins.td (+3-3)
  • (modified) clang/lib/CodeGen/CGBuiltin.cpp (+13)
  • (modified) clang/test/CodeGen/X86/math-builtins.c (+4-4)
  • (modified) clang/test/CodeGen/constrained-math-builtins.c (+13)
  • (modified) clang/test/CodeGen/math-libcalls.c (+6-6)
  • (modified) clang/test/CodeGenOpenCL/builtins-f16.cl (+3)
  • (modified) llvm/docs/LangRef.rst (+36)
  • (modified) llvm/include/llvm/CodeGen/ISDOpcodes.h (+2)
  • (modified) llvm/include/llvm/IR/ConstrainedOps.def (+1)
  • (modified) llvm/include/llvm/IR/Intrinsics.td (+4)
  • (modified) llvm/test/Assembler/fp-intrinsics-attr.ll (+8)
  • (modified) llvm/test/Feature/fp-intrinsics.ll (+11)
diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td
index 11982af3fa609..7bef5fd7ad40f 100644
--- a/clang/include/clang/Basic/Builtins.td
+++ b/clang/include/clang/Basic/Builtins.td
@@ -482,11 +482,11 @@ def SqrtF16F128 : Builtin, F16F128MathTemplate {
   let Prototype = "T(T)";
 }
 
-def TanF128 : Builtin {
-  let Spellings = ["__builtin_tanf128"];
+def TanF16F128 : Builtin, F16F128MathTemplate {
+  let Spellings = ["__builtin_tan"];
   let Attributes = [FunctionWithBuiltinPrefix, NoThrow,
                     ConstIgnoringErrnoAndExceptions];
-  let Prototype = "__float128(__float128)";
+  let Prototype = "T(T)";
 }
 
 def TanhF128 : Builtin {
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 91083c1cfae96..7650a006fffb1 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -2922,6 +2922,19 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
       SetSqrtFPAccuracy(Call);
       return RValue::get(Call);
     }
+
+    case Builtin::BItan:
+    case Builtin::BItanf:
+    case Builtin::BItanl:
+    case Builtin::BI__builtin_tan:
+    case Builtin::BI__builtin_tanf:
+    case Builtin::BI__builtin_tanf16:
+    case Builtin::BI__builtin_tanl:
+    case Builtin::BI__builtin_tanf128:
+      return RValue::get(emitUnaryMaybeConstrainedFPBuiltin(*this, E,
+                                   Intrinsic::tan,
+                                   Intrinsic::experimental_constrained_tan));
+
     case Builtin::BItrunc:
     case Builtin::BItruncf:
     case Builtin::BItruncl:
diff --git a/clang/test/CodeGen/X86/math-builtins.c b/clang/test/CodeGen/X86/math-builtins.c
index 093239b448260..1e0f129b98610 100644
--- a/clang/test/CodeGen/X86/math-builtins.c
+++ b/clang/test/CodeGen/X86/math-builtins.c
@@ -674,10 +674,10 @@ __builtin_sqrt(f);       __builtin_sqrtf(f);      __builtin_sqrtl(f); __builtin_
 
 __builtin_tan(f);        __builtin_tanf(f);       __builtin_tanl(f); __builtin_tanf128(f);
 
-// NO__ERRNO: declare double @tan(double noundef) [[READNONE]]
-// NO__ERRNO: declare float @tanf(float noundef) [[READNONE]]
-// NO__ERRNO: declare x86_fp80 @tanl(x86_fp80 noundef) [[READNONE]]
-// NO__ERRNO: declare fp128 @tanf128(fp128 noundef) [[READNONE]]
+// NO__ERRNO: declare double @llvm.tan.f64(double) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare float @llvm.tan.f32(float) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare x86_fp80 @llvm.tan.f80(x86_fp80) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare fp128 @llvm.tan.f128(fp128) [[READNONE_INTRINSIC]]
 // HAS_ERRNO: declare double @tan(double noundef) [[NOT_READNONE]]
 // HAS_ERRNO: declare float @tanf(float noundef) [[NOT_READNONE]]
 // HAS_ERRNO: declare x86_fp80 @tanl(x86_fp80 noundef) [[NOT_READNONE]]
diff --git a/clang/test/CodeGen/constrained-math-builtins.c b/clang/test/CodeGen/constrained-math-builtins.c
index 2de832dd2b6ca..4673418c79c97 100644
--- a/clang/test/CodeGen/constrained-math-builtins.c
+++ b/clang/test/CodeGen/constrained-math-builtins.c
@@ -183,6 +183,14 @@ void foo(double *d, float f, float *fp, long double *l, int *i, const char *c, _
 // CHECK: call x86_fp80 @llvm.experimental.constrained.sqrt.f80(x86_fp80 %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
 // CHECK: call fp128 @llvm.experimental.constrained.sqrt.f128(fp128 %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
 
+__builtin_tan(f);        __builtin_tanf(f);       __builtin_tanl(f); __builtin_tanf128(f);
+
+// CHECK: call double @llvm.experimental.constrained.tan.f64(double %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+// CHECK: call float @llvm.experimental.constrained.tan.f32(float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+// CHECK: call x86_fp80 @llvm.experimental.constrained.tan.f80(x86_fp80 %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+// CHECK: call fp128 @llvm.experimental.constrained.tan.f128(fp128 %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+
+
   __builtin_trunc(f);      __builtin_truncf(f);     __builtin_truncl(f); __builtin_truncf128(f);
 
 // CHECK: call double @llvm.experimental.constrained.trunc.f64(double %{{.*}}, metadata !"fpexcept.strict")
@@ -315,6 +323,11 @@ void foo(double *d, float f, float *fp, long double *l, int *i, const char *c, _
 // CHECK: declare x86_fp80 @llvm.experimental.constrained.sqrt.f80(x86_fp80, metadata, metadata)
 // CHECK: declare fp128 @llvm.experimental.constrained.sqrt.f128(fp128, metadata, metadata)
 
+// CHECK: declare double @llvm.experimental.constrained.tan.f64(double, metadata, metadata)
+// CHECK: declare float @llvm.experimental.constrained.tan.f32(float, metadata, metadata)
+// CHECK: declare x86_fp80 @llvm.experimental.constrained.tan.f80(x86_fp80, metadata, metadata)
+// CHECK: declare fp128 @llvm.experimental.constrained.tan.f128(fp128, metadata, metadata)
+
 // CHECK: declare double @llvm.experimental.constrained.trunc.f64(double, metadata)
 // CHECK: declare float @llvm.experimental.constrained.trunc.f32(float, metadata)
 // CHECK: declare x86_fp80 @llvm.experimental.constrained.trunc.f80(x86_fp80, metadata)
diff --git a/clang/test/CodeGen/math-libcalls.c b/clang/test/CodeGen/math-libcalls.c
index 29c312ba0ecac..a249182692762 100644
--- a/clang/test/CodeGen/math-libcalls.c
+++ b/clang/test/CodeGen/math-libcalls.c
@@ -662,15 +662,15 @@ void foo(double *d, float f, float *fp, long double *l, int *i, const char *c) {
 
   tan(f);        tanf(f);       tanl(f);
 
-// NO__ERRNO: declare double @tan(double noundef) [[READNONE]]
-// NO__ERRNO: declare float @tanf(float noundef) [[READNONE]]
-// NO__ERRNO: declare x86_fp80 @tanl(x86_fp80 noundef) [[READNONE]]
+// NO__ERRNO: declare double @llvm.tan.f64(double) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare float @llvm.tan.f32(float) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare x86_fp80 @llvm.tan.f80(x86_fp80) [[READNONE_INTRINSIC]]
 // HAS_ERRNO: declare double @tan(double noundef) [[NOT_READNONE]]
 // HAS_ERRNO: declare float @tanf(float noundef) [[NOT_READNONE]]
 // HAS_ERRNO: declare x86_fp80 @tanl(x86_fp80 noundef) [[NOT_READNONE]]
-// HAS_MAYTRAP: declare double @tan(double noundef) [[NOT_READNONE]]
-// HAS_MAYTRAP: declare float @tanf(float noundef) [[NOT_READNONE]]
-// HAS_MAYTRAP: declare x86_fp80 @tanl(x86_fp80 noundef) [[NOT_READNONE]]
+// HAS_MAYTRAP: declare double @llvm.experimental.constrained.tan.f64(
+// HAS_MAYTRAP: declare float @llvm.experimental.constrained.tan.f32(
+// HAS_MAYTRAP: declare x86_fp80 @llvm.experimental.constrained.tan.f80(
 
   tanh(f);       tanhf(f);      tanhl(f);
 
diff --git a/clang/test/CodeGenOpenCL/builtins-f16.cl b/clang/test/CodeGenOpenCL/builtins-f16.cl
index adf7cdde154f5..d7bffdad5c548 100644
--- a/clang/test/CodeGenOpenCL/builtins-f16.cl
+++ b/clang/test/CodeGenOpenCL/builtins-f16.cl
@@ -66,6 +66,9 @@ void test_half_builtins(half h0, half h1, half h2, int i0) {
   // CHECK: call half @llvm.sqrt.f16(half %h0)
   res = __builtin_sqrtf16(h0);
 
+  // CHECK: call half @llvm.tan.f16(half %h0)
+  res = __builtin_tanf16(h0);
+
   // CHECK: call half @llvm.trunc.f16(half %h0)
   res = __builtin_truncf16(h0);
 
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index d2d21c7c4b5e3..e23c4dd7186fa 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -26169,6 +26169,42 @@ same values as the libm ``cos`` functions would, and handles error
 conditions in the same way.
 
 
+'``llvm.experimental.constrained.tan``' Intrinsic
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Syntax:
+"""""""
+
+::
+
+      declare <type>
+      @llvm.experimental.constrained.tan(<type> <op1>,
+                                         metadata <rounding mode>,
+                                         metadata <exception behavior>)
+
+Overview:
+"""""""""
+
+The '``llvm.experimental.constrained.tan``' intrinsic returns the tangent of the
+first operand.
+
+Arguments:
+""""""""""
+
+The first argument and the return type are floating-point numbers of the same
+type.
+
+The second and third arguments specify the rounding mode and exception
+behavior as described above.
+
+Semantics:
+""""""""""
+
+This function returns the tangent of the specified operand, returning the
+same values as the libm ``tan`` functions would, and handles error
+conditions in the same way.
+
+
 '``llvm.experimental.constrained.exp``' Intrinsic
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
diff --git a/llvm/include/llvm/CodeGen/ISDOpcodes.h b/llvm/include/llvm/CodeGen/ISDOpcodes.h
index d8af97957e48e..22062f0efbbda 100644
--- a/llvm/include/llvm/CodeGen/ISDOpcodes.h
+++ b/llvm/include/llvm/CodeGen/ISDOpcodes.h
@@ -415,6 +415,7 @@ enum NodeType {
   STRICT_FLDEXP,
   STRICT_FSIN,
   STRICT_FCOS,
+  STRICT_FTAN,
   STRICT_FEXP,
   STRICT_FEXP2,
   STRICT_FLOG,
@@ -934,6 +935,7 @@ enum NodeType {
   FCBRT,
   FSIN,
   FCOS,
+  FTAN,
   FPOW,
   FPOWI,
   /// FLDEXP - ldexp, inspired by libm (op0 * 2**op1).
diff --git a/llvm/include/llvm/IR/ConstrainedOps.def b/llvm/include/llvm/IR/ConstrainedOps.def
index 41aa44de957f9..a7b37c5cb204d 100644
--- a/llvm/include/llvm/IR/ConstrainedOps.def
+++ b/llvm/include/llvm/IR/ConstrainedOps.def
@@ -95,6 +95,7 @@ DAG_FUNCTION(round,           1, 0, experimental_constrained_round,      FROUND)
 DAG_FUNCTION(roundeven,       1, 0, experimental_constrained_roundeven,  FROUNDEVEN)
 DAG_FUNCTION(sin,             1, 1, experimental_constrained_sin,        FSIN)
 DAG_FUNCTION(sqrt,            1, 1, experimental_constrained_sqrt,       FSQRT)
+DAG_FUNCTION(tan,             1, 1, experimental_constrained_tan,        FTAN)
 DAG_FUNCTION(trunc,           1, 0, experimental_constrained_trunc,      FTRUNC)
 
 // This is definition for fmuladd intrinsic function, that is converted into
diff --git a/llvm/include/llvm/IR/Intrinsics.td b/llvm/include/llvm/IR/Intrinsics.td
index 3019f68083d42..e1c33c7da33a5 100644
--- a/llvm/include/llvm/IR/Intrinsics.td
+++ b/llvm/include/llvm/IR/Intrinsics.td
@@ -1217,6 +1217,10 @@ let IntrProperties = [IntrInaccessibleMemOnly, IntrWillReturn, IntrStrictFP] in
                                                     [ LLVMMatchType<0>,
                                                       llvm_metadata_ty,
                                                       llvm_metadata_ty ]>;
+  def int_experimental_constrained_tan  : DefaultAttrsIntrinsic<[ llvm_anyfloat_ty ],
+                                                    [ LLVMMatchType<0>,
+                                                      llvm_metadata_ty,
+                                                      llvm_metadata_ty ]>;
   def int_experimental_constrained_pow  : DefaultAttrsIntrinsic<[ llvm_anyfloat_ty ],
                                                     [ LLVMMatchType<0>,
                                                       LLVMMatchType<0>,
diff --git a/llvm/test/Assembler/fp-intrinsics-attr.ll b/llvm/test/Assembler/fp-intrinsics-attr.ll
index 6546d1a275c99..613630e1a2b4d 100644
--- a/llvm/test/Assembler/fp-intrinsics-attr.ll
+++ b/llvm/test/Assembler/fp-intrinsics-attr.ll
@@ -85,6 +85,11 @@ define void @func(double %a, double %b, double %c, i32 %i) strictfp {
                                                metadata !"round.dynamic",
                                                metadata !"fpexcept.strict")
 
+  %tan = call double @llvm.experimental.constrained.tan.f64(
+                                               double %a,
+                                               metadata !"round.dynamic",
+                                               metadata !"fpexcept.strict")
+
   %pow = call double @llvm.experimental.constrained.pow.f64(
                                                double %a, double %b,
                                                metadata !"round.dynamic",
@@ -244,6 +249,9 @@ declare double @llvm.experimental.constrained.sin.f64(double, metadata, metadata
 declare double @llvm.experimental.constrained.cos.f64(double, metadata, metadata)
 ; CHECK: @llvm.experimental.constrained.cos.f64({{.*}}) #[[ATTR1]]
 
+declare double @llvm.experimental.constrained.tan.f64(double, metadata, metadata)
+; CHECK: @llvm.experimental.constrained.tan.f64({{.*}}) #[[ATTR1]]
+
 declare double @llvm.experimental.constrained.pow.f64(double, double, metadata, metadata)
 ; CHECK: @llvm.experimental.constrained.pow.f64({{.*}}) #[[ATTR1]]
 
diff --git a/llvm/test/Feature/fp-intrinsics.ll b/llvm/test/Feature/fp-intrinsics.ll
index b92408a1bf1cd..7759813dc2e11 100644
--- a/llvm/test/Feature/fp-intrinsics.ll
+++ b/llvm/test/Feature/fp-intrinsics.ll
@@ -151,6 +151,17 @@ entry:
   ret double %result
 }
 
+; Verify that tan(42.0) isn't simplified when the rounding mode is unknown.
+; CHECK-LABEL: ftan
+; CHECK: call double @llvm.experimental.constrained.tan
+define double @ftan() #0 {
+entry:
+  %result = call double @llvm.experimental.constrained.tan.f64(double 42.0,
+                                               metadata !"round.dynamic",
+                                               metadata !"fpexcept.strict") #0
+  ret double %result
+}
+
 ; Verify that exp(42.0) isn't simplified when the rounding mode is unknown.
 ; CHECK-LABEL: f10
 ; CHECK: call double @llvm.experimental.constrained.exp

Copy link

github-actions bot commented May 24, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@damyanp damyanp left a comment

Choose a reason for hiding this comment

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

IMO it'd be good to have a bit more context in the commit description about why this is being done. If you follow the breadcrumbs from the issue to the thread in the PR I think you can put it together, but a more local summary might be helpful.

clang/test/CodeGen/constrained-math-builtins.c Outdated Show resolved Hide resolved
@farzonl
Copy link
Member Author

farzonl commented May 24, 2024

IMO it'd be good to have a bit more context in the commit description about why this is being done. If you follow the breadcrumbs from the issue to the thread in the PR I think you can put it together, but a more local summary might be helpful.

done

Copy link
Contributor

@coopp coopp left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

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

LGTM

@farzonl farzonl merged commit b15a0a3 into llvm:main May 29, 2024
9 checks passed
@farzonl farzonl deleted the add-tan-constrained-intrinsic branch May 29, 2024 15:16
@ilovepi
Copy link
Contributor

ilovepi commented May 29, 2024

Hi, we're seeing a codegen issue building Fuchsia after this patch. I'm looking now, but so far I'm not sure why this is happening. Would you mind taking a look?

Error:

FAILED: obj/src/graphics/lib/compute/spinel/ext/transform_stack/transform_stack.transform_stack.c.o 
../../prebuilt/third_party/clang/custom/bin/clang -MD -MF obj/src/graphics/lib/compute/spinel/ext/transform_stack/transform_stack.transform_stack.c.o.d -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCPP_REMOVE_TRANSITIVE_INCLUDES -DNDEBUG=1 -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS=1 -I../.. -Igen -I../../src/graphics/lib/compute/spinel/include -I../../src/graphics/lib/compute -fcolor-diagnostics -fcrash-diagnostics-dir=clang-crashreports -fcrash-diagnostics=all -gen-reproducer=error -ffp-contract=off --sysroot=gen/zircon/public/sysroot/cpp --target=x86_64-unknown-fuchsia -ffuchsia-api-level=4293918720 -march=x86-64-v2 -mtune=generic -mbranches-within-32B-boundaries -ffile-compilation-dir=. -no-canonical-prefixes -fomit-frame-pointer -fdata-sections -ffunction-sections -Os -flto -mllvm -wholeprogramdevirt-branch-funnel-threshold=0 -ffat-lto-objects -Xclang -debug-info-kind=constructor -g3 -grecord-gcc-switches -gdwarf-5 -gz=zstd -Wall -Wextra -Wconversion -Wextra-semi -Wimplicit-fallthrough -Wnewline-eof -Wstrict-prototypes -Wwrite-strings -Wno-sign-conversion -Wno-unused-parameter -Wnonportable-system-include-path -Wno-missing-field-initializers -Wno-extra-qualification -Wno-cast-function-type-strict -Wno-cast-function-type-mismatch -Wno-unknown-warning-option -Wno-deprecated-pragma -fvisibility=hidden -Werror -Wa,--fatal-warnings -ftrivial-auto-var-init=pattern -Wthread-safety -Wno-unknown-warning-option -Wno-thread-safety-reference-return -std=c11 -c ../../src/graphics/lib/compute/spinel/ext/transform_stack/transform_stack.c -o obj/src/graphics/lib/compute/spinel/ext/transform_stack/transform_stack.transform_stack.c.o
fatal error: error in backend: Cannot select: intrinsic %llvm.tan
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.	Program arguments: ../../prebuilt/third_party/clang/custom/bin/clang -MD -MF obj/src/graphics/lib/compute/spinel/ext/transform_stack/transform_stack.transform_stack.c.o.d -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCPP_REMOVE_TRANSITIVE_INCLUDES -DNDEBUG=1 -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS=1 -I../.. -Igen -I../../src/graphics/lib/compute/spinel/include -I../../src/graphics/lib/compute -fcolor-diagnostics -fcrash-diagnostics-dir=clang-crashreports -fcrash-diagnostics=all -gen-reproducer=error -ffp-contract=off --sysroot=gen/zircon/public/sysroot/cpp --target=x86_64-unknown-fuchsia -ffuchsia-api-level=4293918720 -march=x86-64-v2 -mtune=generic -mbranches-within-32B-boundaries -ffile-compilation-dir=. -no-canonical-prefixes -fomit-frame-pointer -fdata-sections -ffunction-sections -Os -flto -mllvm -wholeprogramdevirt-branch-funnel-threshold=0 -ffat-lto-objects -Xclang -debug-info-kind=constructor -g3 -grecord-gcc-switches -gdwarf-5 -gz=zstd -Wall -Wextra -Wconversion -Wextra-semi -Wimplicit-fallthrough -Wnewline-eof -Wstrict-prototypes -Wwrite-strings -Wno-sign-conversion -Wno-unused-parameter -Wnonportable-system-include-path -Wno-missing-field-initializers -Wno-extra-qualification -Wno-cast-function-type-strict -Wno-cast-function-type-mismatch -Wno-unknown-warning-option -Wno-deprecated-pragma -fvisibility=hidden -Werror -Wa,--fatal-warnings -ftrivial-auto-var-init=pattern -Wthread-safety -Wno-unknown-warning-option -Wno-thread-safety-reference-return -std=c11 -c ../../src/graphics/lib/compute/spinel/ext/transform_stack/transform_stack.c -o obj/src/graphics/lib/compute/spinel/ext/transform_stack/transform_stack.transform_stack.c.o
1.	<eof> parser at end of file
2.	Code generation
3.	Running pass 'Function Pass Manager' on module '../../src/graphics/lib/compute/spinel/ext/transform_stack/transform_stack.c'.
4.	Running pass 'X86 DAG->DAG Instruction Selection' on function '@spinel_transform_stack_push_skew_x'
#0 0x0000558e5225bc78 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (../../prebuilt/third_party/clang/custom/bin/clang+0x8936c78)
clang: error: clang frontend command failed with exit code 70 (use -v to see invocation)
Fuchsia clang version 19.0.0git (https://llvm.googlesource.com/llvm-project 1de6011c34b185235cd65c2e3fb030015d182968)
Target: x86_64-unknown-fuchsia
Thread model: posix
InstalledDir: ../../prebuilt/third_party/clang/custom/bin
Build config: +assertions
clang: note: diagnostic msg: 
********************

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang: note: diagnostic msg: clang-crashreports/transform_stack-afeed7.c
clang: note: diagnostic msg: clang-crashreports/transform_stack-afeed7.sh
clang: note: diagnostic msg: 

Failing Bot: https://ci.chromium.org/ui/p/fuchsia/builders/ci/clang_toolchain.ci.core.x64-release-subbuild/b8746589504792556897/overview

Logs: https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket/8746589504792556897/+/u/build/ninja/stdout

Reproducer from bot: https://storage.googleapis.com/fuchsia-artifacts/builds/8746589527028907233/build-debug/clang-crashreports/transform_stack-afeed7.tar

@farzonl
Copy link
Member Author

farzonl commented May 29, 2024

Hi, we're seeing a codegen issue building Fuchsia after this patch. I'm looking now, but so far I'm not sure why this is happening. Would you mind taking a look?

Error:
fatal error: error in backend: Cannot select: intrinsic %llvm.tan
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.

  1. Running pass 'X86 DAG->DAG Instruction Selection' on function '@spinel_transform_stack_push_skew_x'
    #0 0x0000558e5225bc78 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (../../prebuilt/third_party/clang/custom/bin/clang+0x8936c78)
    clang: error: clang frontend command failed with exit code 70 (use -v to see invocation)
    Fuchsia clang version 19.0.0git (https://llvm.googlesource.com/llvm-project 1de6011)

Hi @ilovepi, This is interesting because I didn't see anything in x86 dag specific to constraint intrinsics but I suspect exposing tan as an ISDOpcodes is causing the problem. The issue is because x86 DAG instruction selection hasn't landed yet: #90503 and the pass is trying to replace the intrinsic with something else and can't. I'm still waiting on approvals for that part.

Looks like the right course is to revert this change for now.

farzonl added a commit that referenced this pull request May 29, 2024
farzonl added a commit that referenced this pull request May 29, 2024
…trinsic (#93314)" (#93721)

This reverts commit b15a0a3.

This should undo PR: #93314
will need to re-open #91421

wait for #90503 to land
@farzonl farzonl restored the add-tan-constrained-intrinsic branch May 29, 2024 19:37
@farzonl
Copy link
Member Author

farzonl commented May 29, 2024

@ilovepi, How many platforms do you build Fuchsia for? It will be important to know which backends needs support before we try to re-land this.

@ilovepi
Copy link
Contributor

ilovepi commented May 29, 2024

Thanks for the prompt revert. That's a big help.

@ilovepi, How many platforms do you build Fuchsia for? It will be important to know which backends needs support before we try to re-land this.

Fuchsia builds for Aarch64, RISC-V 64, and X86_64. Our toolchain is also used in a lot of embedded contexts (mostly ARM32) but that is separate from Fuchsia's build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 clang:codegen clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category llvm:ir llvm:SelectionDAG SelectionDAGISel as well
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

[HLSL] Add experimental_constrained_tan intrinsic
7 participants