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

riscv64/LTO: lto1: fatal error: target specific builtin not available #1740

Closed
malaterre opened this issue Sep 12, 2023 · 16 comments · Fixed by #1743
Closed

riscv64/LTO: lto1: fatal error: target specific builtin not available #1740

malaterre opened this issue Sep 12, 2023 · 16 comments · Fixed by #1743

Comments

@malaterre
Copy link
Contributor

Seems like default flag: -march=rv64gcv1p0 is conflicting with -flto=auto. Will need to dig further.

ref:

@malaterre
Copy link
Contributor Author

looks like cmake is not passing -march=rv64gcv1p0 around:

Compare:

/usr/bin/c++ -DHWY_SHARED_DEFINE -Dhwy_EXPORTS -I/home/malat/highway-1.0.7 -g -O2 -ffile-prefix-map=/home/malat/highway-1.0.7=. -flto=auto -ffat-lto-objects -fstack-protector-strong -Wformat -Werror=format-security -DHWY_BROKEN_EMU128=0 -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -Wno-builtin-macro-redefined -D__DATE__="redacted" -D__TIMESTAMP__="redacted" -D__TIME__="redacted" -fmerge-all-constants -Wall -Wextra -Wconversion -Wsign-conversion -Wvla -Wnon-virtual-dtor -fmath-errno -fno-exceptions -march=rv64gcv1p0 -Werror -MD -MT CMakeFiles/hwy.dir/hwy/per_target.cc.o -MF CMakeFiles/hwy.dir/hwy/per_target.cc.o.d -o CMakeFiles/hwy.dir/hwy/per_target.cc.o -c /home/malat/highway-1.0.7/hwy/per_target.cc

while:

/usr/bin/c++ -fPIC -g -O2 -ffile-prefix-map=/home/malat/highway-1.0.7=. -flto=auto -ffat-lto-objects -fstack-protector-strong -Wformat -Werror=format-security -DHWY_BROKEN_EMU128=0 -Wdate-time -D_FORTIFY_SOURCE=2 -Wl,--version-script=/home/malat/highway-1.0.7/hwy/hwy.version -flto=auto -ffat-lto-objects -Wl,-z,relro -Wl,-z,now -shared -Wl,-soname,libhwy.so.1 -o libhwy.so.1.0.7 CMakeFiles/hwy.dir/hwy/aligned_allocator.cc.o CMakeFiles/hwy.dir/hwy/nanobenchmark.cc.o CMakeFiles/hwy.dir/hwy/per_target.cc.o CMakeFiles/hwy.dir/hwy/print.cc.o CMakeFiles/hwy.dir/hwy/targets.cc.o CMakeFiles/hwy.dir/hwy/timer.cc.o

@malaterre
Copy link
Contributor Author

setting target_link_options(hwy PRIVATE ${HWY_FLAGS}) seems to go in the right direction

@malaterre
Copy link
Contributor Author

@jan-wassenberg
Copy link
Member

Thanks for reporting. Sounds like a compiler bug. Would you suggest any code changes or documentation updates?

@malaterre
Copy link
Contributor Author

Would you suggest any code changes or documentation updates?

I vaguely remember we had a discussion about HWY_RISCV. I seem to remember you really wanted to be set automatically. I think I suggested to make it behave like the other ARM7 option (up to user decision). This would make it easy to opt out of the automatic -march=rv64gcv1p0 addition...

@jan-wassenberg
Copy link
Member

Yes indeed, I think you mean #838?
And in the meantime, your proposal is to add HWY_CMAKE_RVV that if set, adds -march=rv64gcv1p0 to HWY_FLAGS?

@malaterre
Copy link
Contributor Author

And in the meantime, your proposal is to add HWY_CMAKE_RVV that if set, adds -march=rv64gcv1p0 to HWY_FLAGS?

Yes precisely ! If I understand correctly HWY_CMAKE_RVV is just a hack until clang implement riscv_vector.h intrinsics should be target-gated (donno about gcc status btw). So it makes both ARM7 and RVV option behave very closely.

llvm/llvm-project#56592

@jan-wassenberg
Copy link
Member

Thanks, that makes sense. hm, I had forgotten that we already detect HWY_RISCV in CMake and add that flag to HWY_FLAGS. Is the issue that HWY_RISCV is not being detected correctly in CMake? (by checking __riscv)

Or should we add target_link_options(hwy PRIVATE ${HWY_FLAGS}) as you mention, unconditionally or only for HWY_RISCV?

@malaterre
Copy link
Contributor Author

Thanks, that makes sense. hm, I had forgotten that we already detect HWY_RISCV in CMake and add that flag to HWY_FLAGS. Is the issue that HWY_RISCV is not being detected correctly in CMake? (by checking __riscv)

HWY_RISCV is a little bit more than just checking riscv, it adds (unconditionally) -march=rv64gcv1p0. This is currently required as neither GCC nor CLANG implement proper target-gated riscv_vector intrinsics (AFAIK).

Or should we add target_link_options(hwy PRIVATE ${HWY_FLAGS}) as you mention, unconditionally or only for HWY_RISCV?

That is a rather bad workaround to cope with current GCC-13 limitation. I'd prefer not to go this route.

Fundamentally in the case of dynamic dispatch, the issue is that I do not want to see any -march=xyz compile option being added when I type make hwy. This issue has bitten me in the past (#1271).

I'll try to prepare a patch for HWY_CMAKE_RVV

@malaterre
Copy link
Contributor Author

malaterre commented Sep 13, 2023

Fundamentally in the case of dynamic dispatch, the issue is that I do not want to see any -march=xyz compile option being added when I type make hwy. This issue has bitten me in the past (#1271).

I take that back: I see that per_target.cc is being compiled in the main hwy library... so march compile option is required for RVV at this point in time.

@jan-wassenberg
Copy link
Member

Right. Ah, I see, you wanted to make the existing "always set march=gcv" conditional upon both RISCV and the user-specified flag. That makes sense, thanks for sending the PR!

@k0tran
Copy link
Contributor

k0tran commented Aug 13, 2024

Just tested it out on pioneer
@jan-wassenberg for compiling it required -march in linker flags too so I propose adding this line after CMakeLists:381

      add_link_options(-march=rv64gcv1p0)

This turns on flag for all targets, I tried turning it on for hwy only, but it did not work
If this is fine by you I could make it into PR.

@jan-wassenberg
Copy link
Member

Thanks @k0tran :) As mentioned above, we'd like to eventually get to a state where we don't have any -march, and that pragma attributes take care of enabling V extension.

Clang is not quite ready for this yet, see llvm/llvm-project#56592. But if you make the add_link_options conditional on HWY_CMAKE_RVV, that would be fine and we'd welcome a PR :)

@k0tran
Copy link
Contributor

k0tran commented Aug 14, 2024

Yes @jan-wassenberg , I understand that this is temporary solution, but these bugs are old. Would be nice to have solution at least for now.
I've also ran tests and two of them are failing (see logs below)
As I'm quite new to the project I would like to ask If you have some wild guesses what it may be caused

99% tests passed, 2 tests failed out of 365

Total Test time (real) =   6.12 sec

The following tests FAILED:
    308 - ImageTestGroup/ImageTest.TestAligned/SCALAR  # GetParam() = 4611686018427387904 (ILLEGAL)
    309 - ImageTestGroup/ImageTest.TestUnaligned/SCALAR  # GetParam() = 4611686018427387904 (ILLEGAL)
Errors while running CTest
Output from these tests are in: /usr/src/RPM/BUILD/highway-1.2.0/riscv64-alt-linux/Testing/Temporary/LastTest.log
Use "--rerun-failed --output-on-failure" to re-run the failed cases verbosely.

In LastTest.log (the other test fails similarly):

308/365 Testing: ImageTestGroup/ImageTest.TestAligned/SCALAR  # GetParam() = 4611686018427387904
308/365 Test: ImageTestGroup/ImageTest.TestAligned/SCALAR  # GetParam() = 4611686018427387904
Command: "/usr/src/RPM/BUILD/highway-1.2.0/riscv64-alt-linux/tests/image_test" "--gtest_filter=ImageTestGroup/ImageTest.TestAligned/SCALAR" "--gtest_also_run_disabled_tests"
Directory: /usr/src/RPM/BUILD/highway-1.2.0/riscv64-alt-linux
"ImageTestGroup/ImageTest.TestAligned/SCALAR  # GetParam() = 4611686018427387904" start time: Aug 14 06:32 UTC
Output:
----------------------------------------------------------
WARNING: CPU supports 0x6000000000000000, software requires 0x4000002000000000
WARNING: CPU supports 0x6000000000000000, software requires 0x4000002000000000
Running main() from /usr/src/RPM/BUILD/googletest-1.13.0/googletest/src/gtest_main.cc
Note: Google Test filter = ImageTestGroup/ImageTest.TestAligned/SCALAR
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from ImageTestGroup/ImageTest
[ RUN      ] ImageTestGroup/ImageTest.TestAligned/SCALAR
<end of output>
Test time =   0.02 sec
----------------------------------------------------------
Test Failed.
"ImageTestGroup/ImageTest.TestAligned/SCALAR  # GetParam() = 4611686018427387904" end time: Aug 14 06:32 UTC
"ImageTestGroup/ImageTest.TestAligned/SCALAR  # GetParam() = 4611686018427387904" time elapsed: 00:00:00

It's just falls silently. I'm not sure where to start

@jan-wassenberg
Copy link
Member

I agree the bugs are old. Progress in compilers is a bit uneven :)

The issue is likely identified by 'WARNING: CPU supports 0x6000000000000000'. Bit 37 being zero indicates RVV is not supported, or at least that we are unable to detect it. Are we sure the CPU and OS actually support V?

Some Linux versions require opting in to V, e.g. because they don't save the vector regs in context switches otherwise.

@k0tran
Copy link
Contributor

k0tran commented Aug 16, 2024

Thanks! Actually it's not supported on machine at the moment, but compiler says it does. So it's not related to highway. I will test it on rvv machine though, If I get one at some point

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.

3 participants