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

LLVM backend assertion seen in multiple SPEC CPU 2017 Fortran benchmarks in release_11x branch #11

Closed
mvinay-ctt opened this issue Jan 22, 2021 · 5 comments

Comments

@mvinay-ctt
Copy link
Contributor

mvinay-ctt commented Jan 22, 2021

Test case:

program foo
      real(kind=8) :: val, res1, res2
      read *, val
      res1 = sin(val)
      res2 = cos(val)
      print *, res1, res2
end program foo

Command: flang -O1

Assertion:

$repo/classic-flang-llvm-project/llvm/lib/IR/Type.cpp:618: static llvm::FixedVectorType* llvm::FixedVectorType::get(llvm::Type*, unsigned int): Assertion `isValidElementType(ElementType) && "Element type of a VectorType must " "be an integer, floating point, or " "pointer type."' failed.
@bryanpkc
Copy link
Collaborator

@michalpasztamobica Would you like to take a look at this?

@michalpasztamobica
Copy link
Collaborator

Hi @bryanpkc ,
We've discussed the matter on flang-dev list and I asked @mvinay-ctt to report this here, so it doesn't get lost as we are currently unable to take care of the failures in SPEC CPU 2017.

alokkrsharma pushed a commit to alokkrsharma/classic-flang-llvm-project that referenced this issue Feb 10, 2021
When `Target::GetEntryPointAddress()` calls `exe_module->GetObjectFile()->GetEntryPointAddress()`, and the returned
`entry_addr` is valid, it can immediately be returned.

However, just before that, an `llvm::Error` value has been setup, but in this case it is not consumed before returning, like is done further below in the function.

In https://bugs.freebsd.org/248745 we got a bug report for this, where a very simple test case aborts and dumps core:

```
* thread flang-compiler#1, name = 'testcase', stop reason = breakpoint 1.1
    frame #0: 0x00000000002018d4 testcase`main(argc=1, argv=0x00007fffffffea18) at testcase.c:3:5
   1	int main(int argc, char *argv[])
   2	{
-> 3	    return 0;
   4	}
(lldb) p argc
Program aborted due to an unhandled Error:
Error value was Success. (Note: Success values must still be checked prior to being destroyed).

Thread 1 received signal SIGABRT, Aborted.
thr_kill () at thr_kill.S:3
3	thr_kill.S: No such file or directory.
(gdb) bt
#0  thr_kill () at thr_kill.S:3
flang-compiler#1  0x00000008049a0004 in __raise (s=6) at /usr/src/lib/libc/gen/raise.c:52
flang-compiler#2  0x0000000804916229 in abort () at /usr/src/lib/libc/stdlib/abort.c:67
flang-compiler#3  0x000000000451b5f5 in fatalUncheckedError () at /usr/src/contrib/llvm-project/llvm/lib/Support/Error.cpp:112
flang-compiler#4  0x00000000019cf008 in GetEntryPointAddress () at /usr/src/contrib/llvm-project/llvm/include/llvm/Support/Error.h:267
flang-compiler#5  0x0000000001bccbd8 in ConstructorSetup () at /usr/src/contrib/llvm-project/lldb/source/Target/ThreadPlanCallFunction.cpp:67
flang-compiler#6  0x0000000001bcd2c0 in ThreadPlanCallFunction () at /usr/src/contrib/llvm-project/lldb/source/Target/ThreadPlanCallFunction.cpp:114
flang-compiler#7  0x00000000020076d4 in InferiorCallMmap () at /usr/src/contrib/llvm-project/lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp:97
flang-compiler#8  0x0000000001f4be33 in DoAllocateMemory () at /usr/src/contrib/llvm-project/lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp:604
flang-compiler#9  0x0000000001fe51b9 in AllocatePage () at /usr/src/contrib/llvm-project/lldb/source/Target/Memory.cpp:347
flang-compiler#10 0x0000000001fe5385 in AllocateMemory () at /usr/src/contrib/llvm-project/lldb/source/Target/Memory.cpp:383
flang-compiler#11 0x0000000001974da2 in AllocateMemory () at /usr/src/contrib/llvm-project/lldb/source/Target/Process.cpp:2301
flang-compiler#12 CanJIT () at /usr/src/contrib/llvm-project/lldb/source/Target/Process.cpp:2331
flang-compiler#13 0x0000000001a1bf3d in Evaluate () at /usr/src/contrib/llvm-project/lldb/source/Expression/UserExpression.cpp:190
flang-compiler#14 0x00000000019ce7a2 in EvaluateExpression () at /usr/src/contrib/llvm-project/lldb/source/Target/Target.cpp:2372
flang-compiler#15 0x0000000001ad784c in EvaluateExpression () at /usr/src/contrib/llvm-project/lldb/source/Commands/CommandObjectExpression.cpp:414
flang-compiler#16 0x0000000001ad86ae in DoExecute () at /usr/src/contrib/llvm-project/lldb/source/Commands/CommandObjectExpression.cpp:646
flang-compiler#17 0x0000000001a5e3ed in Execute () at /usr/src/contrib/llvm-project/lldb/source/Interpreter/CommandObject.cpp:1003
flang-compiler#18 0x0000000001a6c4a3 in HandleCommand () at /usr/src/contrib/llvm-project/lldb/source/Interpreter/CommandInterpreter.cpp:1762
flang-compiler#19 0x0000000001a6f98c in IOHandlerInputComplete () at /usr/src/contrib/llvm-project/lldb/source/Interpreter/CommandInterpreter.cpp:2760
flang-compiler#20 0x0000000001a90b08 in Run () at /usr/src/contrib/llvm-project/lldb/source/Core/IOHandler.cpp:548
flang-compiler#21 0x00000000019a6c6a in ExecuteIOHandlers () at /usr/src/contrib/llvm-project/lldb/source/Core/Debugger.cpp:903
flang-compiler#22 0x0000000001a70337 in RunCommandInterpreter () at /usr/src/contrib/llvm-project/lldb/source/Interpreter/CommandInterpreter.cpp:2946
flang-compiler#23 0x0000000001d9d812 in RunCommandInterpreter () at /usr/src/contrib/llvm-project/lldb/source/API/SBDebugger.cpp:1169
flang-compiler#24 0x0000000001918be8 in MainLoop () at /usr/src/contrib/llvm-project/lldb/tools/driver/Driver.cpp:675
flang-compiler#25 0x000000000191a114 in main () at /usr/src/contrib/llvm-project/lldb/tools/driver/Driver.cpp:890```

Fix the incorrect error catch by only instantiating an `Error` object if it is necessary.

Reviewed By: JDevlieghere

Differential Revision: https://reviews.llvm.org/D86355

(cherry picked from commit 1ce07cd)
@michalpasztamobica
Copy link
Collaborator

Hi @bryanpkc , I remember from one of the biweekly syncs, that you have put some effort into checking this issue. Thanks a lot! I was wondering if you had any progress on this so far?

@bryanpkc
Copy link
Collaborator

This is the LLVM IR that causes the assertion on X86:

  %16 = call <{double, double}> @__fd_sincos_1 (double %15), !dbg !18
  .
  .
declare <{double, double}> @__fd_sincos_1(double)

We don't see this on AArch64 since (IIRC) Flang still generates separate sin and cos calls on that architecture.

@shivaramaarao
Copy link
Collaborator

workaround is to use -fveclib=none

there are pgmath related veclib changes added in classic-flang-llvm-project/llvm/lib/Analysis/TargetLibraryInfo.cpp. these changes are causing the assertions.

1 diff -r lib/Analysis/TargetLibraryInfo.cpp /home/amd/cflang/classic-flang-llvm-project/llvm/lib/Analysis/TargetLibraryInfo.cpp
2 30c30,32
3 < "Intel SVML library")));
4 ---
5 > "Intel SVML library"),
6 > clEnumValN(TargetLibraryInfoImpl::PGMATH, "PGMATH",
7 > "PGI math library")));
8 565d566
9 <
10 1567a1569,2018
11 >
12 > // NOTE: All routines listed here are not available on all the architectures.
13 > // Based on the size of vector registers available and the size of data, the
14 > // vector width should be chosen correctly.
15 > case PGMATH: {
16 > const VecDesc VecFuncs[] = {
17 > {"__fd_sin_1", "__fd_sin_2", 2},
...

Keno added a commit to Keno/classic-flang-llvm-project that referenced this issue Aug 14, 2022
As noted in flang-compiler#11,
flang currently crashes when encountering a sincos reference into pgmath.
The issue is is that __fd_sincos_1 is defined as returning a `<{ double, double }>`
struct and there is no LLVM support for automatically vectorizing target
functions of this form. In particular, it is somewhat ambiguous how
to vectorize such function, i.e. how they pack their return values into
the vector registers. `libpgmath` itself also has a somewhat questionable
implementation of the vector forms of `sincos`, relying on this beatuty:
https://github.com/flang-compiler/flang/blob/master/runtime/libpgmath/lib/common/mth_vreturns.c#L8-L47

This may sometimes work in practice, but it is not particularly robust.
For example, this will definitely break in any sort of LTO or
instrumentation setting.

I think until libpgmath is updated and LLVM upstream has a consensus on how
to vectorize these function, we just need to drop trying to vectorize these
functions. As noted, since LLVM was crashing anyway, no performance and
functionality is lost here over current master.

Fixes flang-compiler#11
bryanpkc pushed a commit that referenced this issue Nov 30, 2022
As noted in #11, flang currently crashes when lowering
a sincos reference into a libpgmath runtime function call. The issue is
that `__fd_sincos_1` is defined as returning a `<{ double, double }>`
struct and there is no LLVM support for automatically vectorizing target
functions of this form. In particular, it is somewhat ambiguous how to
vectorize such functions, i.e. how they pack their return values into the
vector registers. libpgmath itself also has a somewhat questionable
implementation of the vector forms of `sincos`, relying on this beauty:

https://github.com/flang-compiler/flang/blob/master/runtime/libpgmath/lib/common/mth_vreturns.c#L8-L47

This may sometimes work in practice, but it is not particularly
robust. For example, this will definitely break in any sort of LTO or
instrumentation setting.

I think until libpgmath is updated and LLVM upstream has a consensus on
how to vectorize these functions, we just need to stop trying to vectorize
these functions. As noted, since LLVM was crashing anyway, no performance
and functionality is lost here over current master.

Fixes #11.

Originally By: Keno Fischer <keno@juliacomputing.com>
bryanpkc pushed a commit to Huawei-CPLLab/classic-flang-llvm-project that referenced this issue Nov 30, 2022
…iler#136)

As noted in flang-compiler#11, flang currently crashes when lowering
a sincos reference into a libpgmath runtime function call. The issue is
that `__fd_sincos_1` is defined as returning a `<{ double, double }>`
struct and there is no LLVM support for automatically vectorizing target
functions of this form. In particular, it is somewhat ambiguous how to
vectorize such functions, i.e. how they pack their return values into the
vector registers. libpgmath itself also has a somewhat questionable
implementation of the vector forms of `sincos`, relying on this beauty:

https://github.com/flang-compiler/flang/blob/master/runtime/libpgmath/lib/common/mth_vreturns.c#L8-L47

This may sometimes work in practice, but it is not particularly
robust. For example, this will definitely break in any sort of LTO or
instrumentation setting.

I think until libpgmath is updated and LLVM upstream has a consensus on
how to vectorize these functions, we just need to stop trying to vectorize
these functions. As noted, since LLVM was crashing anyway, no performance
and functionality is lost here over current master.

Fixes flang-compiler#11.

Originally By: Keno Fischer <keno@juliacomputing.com>
bryanpkc pushed a commit to Huawei-CPLLab/classic-flang-llvm-project that referenced this issue Nov 30, 2022
As noted in flang-compiler#11, flang currently crashes when lowering
a sincos reference into a libpgmath runtime function call. The issue is
that `__fd_sincos_1` is defined as returning a `<{ double, double }>`
struct and there is no LLVM support for automatically vectorizing target
functions of this form. In particular, it is somewhat ambiguous how to
vectorize such functions, i.e. how they pack their return values into the
vector registers. libpgmath itself also has a somewhat questionable
implementation of the vector forms of `sincos`, relying on this beauty:

https://github.com/flang-compiler/flang/blob/master/runtime/libpgmath/lib/common/mth_vreturns.c#L8-L47

This may sometimes work in practice, but it is not particularly
robust. For example, this will definitely break in any sort of LTO or
instrumentation setting.

I think until libpgmath is updated and LLVM upstream has a consensus on
how to vectorize these functions, we just need to stop trying to vectorize
these functions. As noted, since LLVM was crashing anyway, no performance
and functionality is lost here over current master.

Fixes flang-compiler#11.

Originally By: Keno Fischer <keno@juliacomputing.com>
kaadam pushed a commit to kaadam/classic-flang-llvm-project that referenced this issue Jan 4, 2023
We experienced some deadlocks when we used multiple threads for logging
using `scan-builds` intercept-build tool when we used multiple threads by
e.g. logging `make -j16`

```
(gdb) bt
#0  0x00007f2bb3aff110 in __lll_lock_wait () from /lib/x86_64-linux-gnu/libpthread.so.0
flang-compiler#1  0x00007f2bb3af70a3 in pthread_mutex_lock () from /lib/x86_64-linux-gnu/libpthread.so.0
flang-compiler#2  0x00007f2bb3d152e4 in ?? ()
flang-compiler#3  0x00007ffcc5f0cc80 in ?? ()
flang-compiler#4  0x00007f2bb3d2bf5b in ?? () from /lib64/ld-linux-x86-64.so.2
flang-compiler#5  0x00007f2bb3b5da27 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
flang-compiler#6  0x00007f2bb3b5dbe0 in exit () from /lib/x86_64-linux-gnu/libc.so.6
flang-compiler#7  0x00007f2bb3d144ee in ?? ()
flang-compiler#8  0x746e692f706d742f in ?? ()
flang-compiler#9  0x692d747065637265 in ?? ()
flang-compiler#10 0x2f653631326b3034 in ?? ()
flang-compiler#11 0x646d632e35353532 in ?? ()
flang-compiler#12 0x0000000000000000 in ?? ()
```

I think the gcc's exit call caused the injected `libear.so` to be unloaded
by the `ld`, which in turn called the `void on_unload() __attribute__((destructor))`.
That tried to acquire an already locked mutex which was left locked in the
`bear_report_call()` call, that probably encountered some error and
returned early when it forgot to unlock the mutex.

All of these are speculation since from the backtrace I could not verify
if frames 2 and 3 are in fact corresponding to the `libear.so` module.
But I think it's a fairly safe bet.

So, hereby I'm releasing the held mutex on *all paths*, even if some failure
happens.

PS: I would use lock_guards, but it's C.

Reviewed-by: NoQ

Differential Revision: https://reviews.llvm.org/D118439

(cherry picked from commit d919d02)
bryanpkc pushed a commit to Huawei-CPLLab/classic-flang-llvm-project that referenced this issue Jul 4, 2023
As noted in flang-compiler#11, flang currently crashes when lowering
a sincos reference into a libpgmath runtime function call. The issue is
that `__fd_sincos_1` is defined as returning a `<{ double, double }>`
struct and there is no LLVM support for automatically vectorizing target
functions of this form. In particular, it is somewhat ambiguous how to
vectorize such functions, i.e. how they pack their return values into the
vector registers. libpgmath itself also has a somewhat questionable
implementation of the vector forms of `sincos`, relying on this beauty:

https://github.com/flang-compiler/flang/blob/master/runtime/libpgmath/lib/common/mth_vreturns.c#L8-L47

This may sometimes work in practice, but it is not particularly
robust. For example, this will definitely break in any sort of LTO or
instrumentation setting.

I think until libpgmath is updated and LLVM upstream has a consensus on
how to vectorize these functions, we just need to stop trying to vectorize
these functions. As noted, since LLVM was crashing anyway, no performance
and functionality is lost here over current master.

Fixes flang-compiler#11.

Originally By: Keno Fischer <keno@juliacomputing.com>
bryanpkc pushed a commit that referenced this issue Jul 6, 2023
As noted in #11, flang currently crashes when lowering
a sincos reference into a libpgmath runtime function call. The issue is
that `__fd_sincos_1` is defined as returning a `<{ double, double }>`
struct and there is no LLVM support for automatically vectorizing target
functions of this form. In particular, it is somewhat ambiguous how to
vectorize such functions, i.e. how they pack their return values into the
vector registers. libpgmath itself also has a somewhat questionable
implementation of the vector forms of `sincos`, relying on this beauty:

https://github.com/flang-compiler/flang/blob/master/runtime/libpgmath/lib/common/mth_vreturns.c#L8-L47

This may sometimes work in practice, but it is not particularly
robust. For example, this will definitely break in any sort of LTO or
instrumentation setting.

I think until libpgmath is updated and LLVM upstream has a consensus on
how to vectorize these functions, we just need to stop trying to vectorize
these functions. As noted, since LLVM was crashing anyway, no performance
and functionality is lost here over current master.

Fixes #11.

Originally By: Keno Fischer <keno@juliacomputing.com>
bryanpkc pushed a commit to Huawei-CPLLab/classic-flang-llvm-project that referenced this issue Oct 5, 2023
As noted in flang-compiler#11, flang currently crashes when lowering
a sincos reference into a libpgmath runtime function call. The issue is
that `__fd_sincos_1` is defined as returning a `<{ double, double }>`
struct and there is no LLVM support for automatically vectorizing target
functions of this form. In particular, it is somewhat ambiguous how to
vectorize such functions, i.e. how they pack their return values into the
vector registers. libpgmath itself also has a somewhat questionable
implementation of the vector forms of `sincos`, relying on this beauty:

https://github.com/flang-compiler/flang/blob/master/runtime/libpgmath/lib/common/mth_vreturns.c#L8-L47

This may sometimes work in practice, but it is not particularly
robust. For example, this will definitely break in any sort of LTO or
instrumentation setting.

I think until libpgmath is updated and LLVM upstream has a consensus on
how to vectorize these functions, we just need to stop trying to vectorize
these functions. As noted, since LLVM was crashing anyway, no performance
and functionality is lost here over current master.

Fixes flang-compiler#11.

Originally By: Keno Fischer <keno@juliacomputing.com>
bryanpkc pushed a commit that referenced this issue Oct 18, 2023
As noted in #11, flang currently crashes when lowering
a sincos reference into a libpgmath runtime function call. The issue is
that `__fd_sincos_1` is defined as returning a `<{ double, double }>`
struct and there is no LLVM support for automatically vectorizing target
functions of this form. In particular, it is somewhat ambiguous how to
vectorize such functions, i.e. how they pack their return values into the
vector registers. libpgmath itself also has a somewhat questionable
implementation of the vector forms of `sincos`, relying on this beauty:

https://github.com/flang-compiler/flang/blob/master/runtime/libpgmath/lib/common/mth_vreturns.c#L8-L47

This may sometimes work in practice, but it is not particularly
robust. For example, this will definitely break in any sort of LTO or
instrumentation setting.

I think until libpgmath is updated and LLVM upstream has a consensus on
how to vectorize these functions, we just need to stop trying to vectorize
these functions. As noted, since LLVM was crashing anyway, no performance
and functionality is lost here over current master.

Fixes #11.

Originally By: Keno Fischer <keno@juliacomputing.com>
bryanpkc pushed a commit to Huawei-CPLLab/classic-flang-llvm-project that referenced this issue Mar 20, 2024
As noted in flang-compiler#11, flang currently crashes when lowering
a sincos reference into a libpgmath runtime function call. The issue is
that `__fd_sincos_1` is defined as returning a `<{ double, double }>`
struct and there is no LLVM support for automatically vectorizing target
functions of this form. In particular, it is somewhat ambiguous how to
vectorize such functions, i.e. how they pack their return values into the
vector registers. libpgmath itself also has a somewhat questionable
implementation of the vector forms of `sincos`, relying on this beauty:

https://github.com/flang-compiler/flang/blob/master/runtime/libpgmath/lib/common/mth_vreturns.c#L8-L47

This may sometimes work in practice, but it is not particularly
robust. For example, this will definitely break in any sort of LTO or
instrumentation setting.

I think until libpgmath is updated and LLVM upstream has a consensus on
how to vectorize these functions, we just need to stop trying to vectorize
these functions. As noted, since LLVM was crashing anyway, no performance
and functionality is lost here over current master.

Fixes flang-compiler#11.

Originally By: Keno Fischer <keno@juliacomputing.com>
bryanpkc pushed a commit to Huawei-CPLLab/classic-flang-llvm-project that referenced this issue Mar 25, 2024
As noted in flang-compiler#11, flang currently crashes when lowering
a sincos reference into a libpgmath runtime function call. The issue is
that `__fd_sincos_1` is defined as returning a `<{ double, double }>`
struct and there is no LLVM support for automatically vectorizing target
functions of this form. In particular, it is somewhat ambiguous how to
vectorize such functions, i.e. how they pack their return values into the
vector registers. libpgmath itself also has a somewhat questionable
implementation of the vector forms of `sincos`, relying on this beauty:

https://github.com/flang-compiler/flang/blob/master/runtime/libpgmath/lib/common/mth_vreturns.c#L8-L47

This may sometimes work in practice, but it is not particularly
robust. For example, this will definitely break in any sort of LTO or
instrumentation setting.

I think until libpgmath is updated and LLVM upstream has a consensus on
how to vectorize these functions, we just need to stop trying to vectorize
these functions. As noted, since LLVM was crashing anyway, no performance
and functionality is lost here over current master.

Fixes flang-compiler#11.

Originally By: Keno Fischer <keno@juliacomputing.com>
bryanpkc pushed a commit that referenced this issue Apr 17, 2024
As noted in #11, flang currently crashes when lowering
a sincos reference into a libpgmath runtime function call. The issue is
that `__fd_sincos_1` is defined as returning a `<{ double, double }>`
struct and there is no LLVM support for automatically vectorizing target
functions of this form. In particular, it is somewhat ambiguous how to
vectorize such functions, i.e. how they pack their return values into the
vector registers. libpgmath itself also has a somewhat questionable
implementation of the vector forms of `sincos`, relying on this beauty:

https://github.com/flang-compiler/flang/blob/master/runtime/libpgmath/lib/common/mth_vreturns.c#L8-L47

This may sometimes work in practice, but it is not particularly
robust. For example, this will definitely break in any sort of LTO or
instrumentation setting.

I think until libpgmath is updated and LLVM upstream has a consensus on
how to vectorize these functions, we just need to stop trying to vectorize
these functions. As noted, since LLVM was crashing anyway, no performance
and functionality is lost here over current master.

Fixes #11.

Originally By: Keno Fischer <keno@juliacomputing.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants