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

SVC_ME_FunTest failed on loongarch64 platform with openh264 2.5.0 #3857

Open
wszqkzqk opened this issue Feb 12, 2025 · 5 comments
Open

SVC_ME_FunTest failed on loongarch64 platform with openh264 2.5.0 #3857

wszqkzqk opened this issue Feb 12, 2025 · 5 comments

Comments

@wszqkzqk
Copy link

wszqkzqk commented Feb 12, 2025

With openh264 2.5.0 or 2.6.0's test process, SVC_ME_FunTest failed on loongarch64 platform:

test/encoder/EncUT_SVC_me.cpp:143: Failure
Expected equality of these values:
  iRes[0]
    Which is: 7850
  iRes[1]
    Which is: 8361
[  FAILED  ] SVC_ME_FunTest.SumOf8x8SingleBlock_lsx (0 ms)
@wszqkzqk
Copy link
Author

wszqkzqk commented Feb 22, 2025

Additional info: This error only occur while built with LTO (either bfd or mold).

/cc @jinboson @wangluls

@jinboson
Copy link
Contributor

Thanks, we will check what's going on.

@wszqkzqk
Copy link
Author

wszqkzqk commented Mar 2, 2025

There is a warning in LTO:

./codec/processing/src/vaacalc/vaacalculation.h:148:25: warning: type of ‘VAACalcSad_lsx’ does not match original declaration [-Wlto-type-mismatch]
  148 | VAACalcSadFunc          VAACalcSad_lsx;
      |                         ^
codec/processing/src/loongarch/vaa_lsx.c:46:6: note: type mismatch in parameter 8
   46 | void VAACalcSad_lsx (const uint8_t* pCurData, const uint8_t* pRefData,
      |      ^
codec/processing/src/loongarch/vaa_lsx.c:46:6: note: ‘VAACalcSad_lsx’ was previously declared here

@xry111
Copy link

xry111 commented Mar 2, 2025

There is a warning in LTO:

./codec/processing/src/vaacalc/vaacalculation.h:148:25: warning: type of ‘VAACalcSad_lsx’ does not match original declaration [-Wlto-type-mismatch]
  148 | VAACalcSadFunc          VAACalcSad_lsx;
      |                         ^
codec/processing/src/loongarch/vaa_lsx.c:46:6: note: type mismatch in parameter 8
   46 | void VAACalcSad_lsx (const uint8_t* pCurData, const uint8_t* pRefData,
      |      ^
codec/processing/src/loongarch/vaa_lsx.c:46:6: note: ‘VAACalcSad_lsx’ was previously declared here

I submitted #3864 to fix this, but it's not enough to fix the test failure.

I've spotted a compiler bug related to __lsx_vldx which is used by SumOf8x8SingleBlock_lsx: https://gcc.gnu.org/PR119084 but I don't know if the failure is really caused by the compiler bug.

@xry111
Copy link

xry111 commented Mar 3, 2025

I've spotted a compiler bug related to __lsx_vldx which is used by SumOf8x8SingleBlock_lsx: https://gcc.gnu.org/PR119084 but I don't know if the failure is really caused by the compiler bug.

Building openh264 with a patched GCC has fixed the issue.

sebhub pushed a commit to RTEMS/gnu-mirror-gcc that referenced this issue Mar 5, 2025
…9084]

They could be incorrectly reordered with store instructions like st.b
because the RTL expression does not have a memory_operand or a (mem)
expression.  The incorrect reorder has been observed in openh264 LTO
build.

Expand them to a (mem) expression instead of unspec to fix the issue.
Then we need to make loongarch_address_insns return 1 for
ADDRESS_REG_REG because the constraint "R" expects this behavior, or
the vldx instruction will be considered invalid by the register
allocate pass and turned to add.d + vld.  Apply the ADDRESS_REG_REG
penalty in loongarch_address_cost instead, loongarch_rtx_costs should
also call loongarch_address_cost instead of loongarch_address_insns
then.

Closes: cisco/openh264#3857

gcc/ChangeLog:

	PR target/119084
	* config/loongarch/lasx.md (UNSPEC_LASX_XVLDX): Remove.
	(lasx_xvldx): Remove.
	* config/loongarch/lsx.md (UNSPEC_LSX_VLDX): Remove.
	(lsx_vldx): Remove.
	* config/loongarch/simd.md (QIVEC): New define_mode_iterator.
	(<simd_isa>_<x>vldx): New define_expand.
	* config/loongarch/loongarch.cc (loongarch_address_insns_1): New
	static function with most logic factored out from ...
	(loongarch_address_insns): ... here.  Call
	loongarch_address_insns_1 with reg_reg_cost = 1.
	(loongarch_address_cost): Call loongarch_address_insns_1 with
	reg_reg_cost = la_addr_reg_reg_cost.

gcc/testsuite/ChangeLog:

	PR target/119084
	* gcc.target/loongarch/pr119084.c: New test.
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

No branches or pull requests

3 participants