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.

Sign up for GitHub

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

Jump to bottom

Add loongarch64 support to bridges #3

Merged
merged 1 commit into from Jul 21, 2022
Merged

Add loongarch64 support to bridges #3

merged 1 commit into from Jul 21, 2022

Conversation

wjh-la
Copy link

@wjh-la wjh-la commented Jul 8, 2022

No description provided.

xen0n
xen0n reviewed Jul 16, 2022
View reviewed changes
solenv/clang-format/excludelist Outdated
Comment on lines 451 to 454
bridges/source/cpp_uno/gcc3_linux_loongarch64/abi.cxx
bridges/source/cpp_uno/gcc3_linux_loongarch64/abi.hxx
bridges/source/cpp_uno/gcc3_linux_loongarch64/cpp2uno.cxx
bridges/source/cpp_uno/gcc3_linux_loongarch64/uno2cpp.cxx
Copy link

Choose a reason for hiding this comment

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

The list is sorted alphabetically, you should add the lines somewhere below.

Copy link

Choose a reason for hiding this comment

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

Actually, new code should get properly formatted.

During local testing and tweaking of this code, I noticed an awful lot of style inconsistencies, so you'd be better off just clang-formatting all of the files.

bridges/source/cpp_uno/gcc3_linux_loongarch64/cpp2uno.cxx Outdated
Comment on lines 384 to 394
// load vtableSlotCall to $t8
reinterpret_cast<unsigned int *>(code)[4] = 0x14000014
| (((((unsigned long)vtableSlotCall) >> 12) & 0x000fffff) << 5);
reinterpret_cast<unsigned int *>(code)[5] = 0x03800294
| ((((unsigned long)vtableSlotCall) & 0x00000fff) << 10);
reinterpret_cast<unsigned int *>(code)[6] = 0x16000014
| (((((unsigned long)vtableSlotCall) >> 32) & 0x000fffff) << 5);
reinterpret_cast<unsigned int *>(code)[7] = 0x03000294
| (((((unsigned long)vtableSlotCall) >> 52) & 0x00000fff) << 10);
// jump $t8
reinterpret_cast<unsigned int *>(code)[8] = 0x4c000280;
Copy link

Choose a reason for hiding this comment

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

mention the assembly in comments for these insns, similar to the 4 insns above?

Copy link

Choose a reason for hiding this comment

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

Actually these are the lu12i.w + ori + lu32i.d + lu52i.d plus jr $t8.

Copy link
Author

Choose a reason for hiding this comment

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

I have revised it again

bridges/source/cpp_uno/gcc3_linux_loongarch64/uno2cpp.cxx Outdated
"ld.d $r9, %[gpr], 40\n\t"
"ld.d $r10, %[gpr], 48\n\t"
"ld.d $r11, %[gpr], 56\n\t"
// Fill the floating pointer registers
Copy link

Choose a reason for hiding this comment

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

Suggested change
// Fill the floating pointer registers
// Fill the floating-point registers

bridges/source/cpp_uno/gcc3_linux_loongarch64/uno2cpp.cxx Outdated
"move %[gret1], $r4 \n\t"
"move %[gret2], $r5 \n\t"
"fmov.d %[fret1], $f0 \n\t"
"fmov.d %[fret2], $f1 \n\t"
Copy link

Choose a reason for hiding this comment

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

remove the stray space before \n\t

@xen0n xen0n mentioned this pull request Jul 17, 2022
Closed
xen0n
xen0n suggested changes Jul 17, 2022
View reviewed changes
bridges/source/cpp_uno/gcc3_linux_loongarch64/cpp2uno.cxx Outdated
Comment on lines 307 to 313
struct loongarch64_va_list {
void * stack;
void * gr_top;
void * vr_top;
int gr_offs;
int vr_offs;
};
Copy link

Choose a reason for hiding this comment

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

I have verified this is wrong. The LoongArch gcc port defines the va_list to be just a pointer, while you copied from the AArch64 port.

bridges/source/cpp_uno/gcc3_linux_loongarch64/cpp2uno.cxx Outdated
Comment on lines 384 to 394
// load vtableSlotCall to $t8
reinterpret_cast<unsigned int *>(code)[4] = 0x14000014
| (((((unsigned long)vtableSlotCall) >> 12) & 0x000fffff) << 5);
reinterpret_cast<unsigned int *>(code)[5] = 0x03800294
| ((((unsigned long)vtableSlotCall) & 0x00000fff) << 10);
reinterpret_cast<unsigned int *>(code)[6] = 0x16000014
| (((((unsigned long)vtableSlotCall) >> 32) & 0x000fffff) << 5);
reinterpret_cast<unsigned int *>(code)[7] = 0x03000294
| (((((unsigned long)vtableSlotCall) >> 52) & 0x00000fff) << 10);
// jump $t8
reinterpret_cast<unsigned int *>(code)[8] = 0x4c000280;
Copy link

Choose a reason for hiding this comment

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

Actually these are the lu12i.w + ori + lu32i.d + lu52i.d plus jr $t8.

bridges/source/cpp_uno/gcc3_linux_loongarch64/cpp2uno.cxx Outdated
Comment on lines 468 to 471
static void (*clear_cache)(unsigned char const *, unsigned char const *)
= (void (*)(unsigned char const *, unsigned char const *)) dlsym(
RTLD_DEFAULT, "__clear_cache");
(*clear_cache)(begin, end);
Copy link

Choose a reason for hiding this comment

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

This is again wrong. __clear_cache is no-op on LoongArch, as the LoongArch gcc doesn't define CLEAR_INSN_CACHE for libgcc.

A simple ibar 0 should be enough.

solenv/clang-format/excludelist Outdated
Comment on lines 451 to 454
bridges/source/cpp_uno/gcc3_linux_loongarch64/abi.cxx
bridges/source/cpp_uno/gcc3_linux_loongarch64/abi.hxx
bridges/source/cpp_uno/gcc3_linux_loongarch64/cpp2uno.cxx
bridges/source/cpp_uno/gcc3_linux_loongarch64/uno2cpp.cxx
Copy link

Choose a reason for hiding this comment

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

Actually, new code should get properly formatted.

During local testing and tweaking of this code, I noticed an awful lot of style inconsistencies, so you'd be better off just clang-formatting all of the files.

@sunhaiyong1978 sunhaiyong1978 merged commit 38553e7 into loongson:loongarch-dev Jul 21, 2022
sunhaiyong1978 pushed a commit that referenced this pull request Jul 21, 2022
@stbergmann
Make combineScanlineChannels stop before padding bytes
eda0c48 
At <https://ci.libreoffice.org/job/lo_ubsan/2467>,
CppunitTest_sd_export_tests-ooxml1 failed with

> ==4831==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x629000211c54 at pc 0x7fcdcb44093f bp 0x7ffe85792760 sp 0x7ffe85792758
> READ of size 1 at 0x629000211c54 thread T0
>     #0 0x7fcdcb44093e in (anonymous namespace)::combineScanlineChannels(unsigned char*, unsigned char*, unsigned char*, unsigned int) /vcl/source/filter/png/PngImageWriter.cxx:27:22
>     #1 0x7fcdcb43fbaf in vcl::pngWrite(SvStream&, BitmapEx const&, int, bool, bool, std::__debug::vector<vcl::PngChunk, std::allocator<vcl::PngChunk> > const&) /vcl/source/filter/png/PngImageWriter.cxx:231:21
>     #2 0x7fcdcb43ce80 in vcl::PngImageWriter::write(BitmapEx const&) /vcl/source/filter/png/PngImageWriter.cxx:318:12
>     #3 0x7fcdcaf04bc1 in GraphicFilter::ExportGraphic(Graphic const&, std::basic_string_view<char16_t, std::char_traits<char16_t> >, SvStream&, unsigned short, com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const*) /vcl/source/filter/graphicfilter.cxx:1801:28
> 0x629000211c54 is located 0 bytes to the right of 19028-byte region [0x62900020d200,0x629000211c54)
> allocated by thread T0 here:
>     #0 0x4fd898 in operator new[](unsigned long) /home/tdf/lode/packages/llvm-llvmorg-12.0.1.src/compiler-rt/lib/asan/asan_new_delete.cpp:102
>     #1 0x7fcdcbcbd50b in ImplCreateDIB(Size const&, vcl::PixelFormat, BitmapPalette const&) /vcl/headless/svpbmp.cxx:123:24
>     #2 0x7fcdcbcbb483 in SvpSalBitmap::Create(Size const&, vcl::PixelFormat, BitmapPalette const&) /vcl/headless/svpbmp.cxx:152:13
>     #3 0x7fcdca406c59 in Bitmap::Bitmap(Size const&, vcl::PixelFormat, BitmapPalette const*) /vcl/source/bitmap/bitmap.cxx:136:15

because for the given N24BitTcBgr bitmap of size 89x71 we have
pAccess->GetScanlineSize() = 268 = 89 * 3 + 1, so combineScanlineChannels wanted
to erroneously read an excessive 90th RGB triplet.

Change-Id: Ida117999de075b8906f43bfe4c2b7fa98df80b0f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/137261
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
@xen0n
Copy link

xen0n commented Jul 22, 2022

Still not working.

$ ./workdir/CustomTarget/testtools/bridgetest/bridgetest_server 

> accepting socket,host=127.0.0.1,port=2002...connection established.warn:sal.osl:1145227:1145278:sal/osl/unx/socket.cxx:1570: receive socket [0] failed: EOL
warn:binaryurp:1145227:1145278:binaryurp/source/reader.cxx:124: caught UNO exception 'N3com3sun4star3uno9ExceptionE msg: acc_socket.cxx:SocketConnection::read: error - Success at /home/xenon/src/libreoffice/core-rel/io/source/acceptor/acc_socket.cxx:175'

$ ./workdir/CustomTarget/testtools/bridgetest/bridgetest_client 
./workdir/CustomTarget/testtools/bridgetest/bridgetest_client: line 1: 1145276 Segmentation fault      (core dumped) /home/xenon/src/libreoffice/core-rel/instdir/program/uno.bin -s com.sun.star.test.bridge.BridgeTest -- -u 'uno:socket,host=127.0.0.1,port=2002;urp;test' -env:LO_BUILD_LIB_DIR=file:///home/xenon/src/libreoffice/core-rel/workdir/LinkTarget/Library -env:URE_MORE_SERVICES=file:///home/xenon/src/libreoffice/core-rel/workdir/Rdb/uno_services.rdb -env:URE_MORE_TYPES=file:///home/xenon/src/libreoffice/core-rel/workdir/UnoApiTarget/bridgetest.rdb

And of course the build doesn't work (no window shown after splash progress reaching 100%).

@xen0n
Copy link

xen0n commented Jul 22, 2022

Hmm, it actually works, I was using remote X11 so the UI is excessively slow but eventually it appeared.

Better fix the bridge test before upstreaming though...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xen0n xen0n xen0n requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@wjh-la @xen0n @sunhaiyong1978