On 9/29/24 11:30 PM, Jovan Vukic wrote:
Based on the feedback I received, the patch now uses a custom code iterator
instead of relying on match_operator.
Since both operands (2 and 3) have trailing zeros, an additional check
was introduced to verify if they remain SMALL_OPERANDs after shifting by
the smaller number of trailing zeros.
To avoid complex inline check, I introduced two new macros, ensuring that the
check is encapsulated and reusable, as suggested in the original feedback.
It was pointed out that this check should be added because expressions like
(x & 0x81000) == 0x8100 or (x & 0x8100) == 0x81000 might lead to
unrecognized instructions during splitting. However, both expressions
immediately evaluate to false (https://godbolt.org/z/Y11EGMb4f) due to
the bit arrangement in constants 0x8100 and 0x81000.
As a result, this pattern won't be applied in such cases, so it cannot
cause any issues. Therefore, it wasn't necessary to include this as a
negative test in the testsuite.
Despite my efforts, I couldn't find another example for a negative test.
All similar cases evaluated to false immediately due to the bit arrangement.
I mentioned this in a follow-up comment on the previous patch version.
Regardless, the necessary check has been added, so if a negative case
exists, an unrecognized instruction will not be created.
2024-09-30 Jovan Vukic <jovan.vu...@rt-rk.com>
PR target/115921
gcc/ChangeLog:
* config/riscv/iterators.md (any_eq): New code iterator.
* config/riscv/riscv.h (COMMON_TRAILING_ZEROS): New macro.
(SMALL_AFTER_COMMON_TRAILING_SHIFT): Ditto.
* config/riscv/riscv.md
(*branch<ANYI:mode>_shiftedarith_<optab>_shifted):
New pattern.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/branch-1.c: Additional tests.
So as I mentioned in the meeting today, I think the patch is fine from a
correctness standpoint. But what we don't have is a test showing an
improvement from this change.
The included test shows a change, but the resulting code isn't
necessarily actually better.
Before:
f5:
li a5,34734080
and a0,a0,a5
li a5,33554432
beq a0,a5,.L4
After:
f5:
srli a5,a0,17
andi a5,a5,265
li a4,256
beq a5,a4,.L4
So yes the code changed and we reduced the constants, but I wouldn't
expect it to run any faster. The same number of instructions and the
same number of data dependencies.
Jeff
