On 04/03/2025 14:49, Torbjorn SVENSSON wrote:
>
>
> On 2025-03-03 18:00, Richard Earnshaw wrote:
>> On 28/02/2025 16:18, Richard Earnshaw wrote:
>>> On 28/02/2025 16:12, Richard Earnshaw wrote:
>>>> On 08/11/2024 18:47, Torbjörn SVENSSON wrote:
>>>>> Ok for trunk and releases/gcc-14?
>>>>>
>>>>> --
>>>>>
>>>>> A long time ago, this test forced -march=armv6.
>>>>>
>>>>> With -marm, the generated assembler is:
>>>>> foo:
>>>>> sub r0, r0, #48
>>>>> cmp r0, #9
>>>>> movhi r0, #0
>>>>> movls r0, #1
>>>>> bx lr
>>>>>
>>>>> With -mthumb, the generated assembler is:
>>>>> foo:
>>>>> subs r0, r0, #48
>>>>> movs r2, #9
>>>>> uxtb r3, r0
>>>>> movs r0, #0
>>>>> cmp r2, r3
>>>>> adcs r0, r0, r0
>>>>> uxtb r0, r0
>>>>> bx lr
>>>>>
>>>>
>>>> Looking at this code, I think both the uxtb instructions are unnecessary.
>>>>
>>>> For the first UXTB. On entry, 'c' is a char (unsigned) so the value is
>>>> passed by the caller in a 32-bit reg, but range-limited (by the ABI) to
>>>> values between 0 and 255.
>>>> We subtract 48 from that, so the range now, is from -48 to 207, but we're
>>>> going to look at this as an unsigned value, so it's effectively 0..207 or
>>>> UINT_MAX-48..UINT_MAX.
>>>> The UXTB instruction then converts the range so that the range becomes
>>>> 0..255 again (but importantly, the values between UINT_MAX-48 and UINT_MAX
>>>> are mapped to the range 208..255. We then do an unsigned comparison
>>>> between that value and the constant 9 to test whether the result is >= 9.
>>>> Now, importantly, all the values that are in the range 208..255 are >= 9,
>>>> but were also >= 9 before the UXTB instruction. In effect, the UXTB is
>>>> completely redundant.
>>>>
>>> I said, >= 9 above, but it should be > 9 (hence condition code 'hi'). That
>>> does affect the rest of the argument.
>>>
>>> R.
>>>
>>>> The second UXTB is even more egregious. We have 0 in r0 and we then
>>>> conditionally add 1 to it, so r0 is in the range 0..1. Zero extending
>>>> that with a UXTB is therefore clearly pointless.
>>>>
>>>> So neither of the UXTB instructions should be present, even if generating
>>>> Thumb1 code.
>>>>
>>>> I think the failures are, therefore, real and we should look to fix the
>>>> compiler rather than 'fix' the scope of the test.
>>>>
>>>> R.
>>>>
>>>>> Require effective-target arm_arm_ok to skip the test for thumb-only
>>>>> targets (Cortex-M).
>>>>>
>>>>> gcc/testsuite/ChangeLog:
>>>>>
>>>>> * gcc.target/arm/unsigned-extend-1.c: Use effective-target
>>>>> arm_arm_ok.
>>>>>
>>>>> Signed-off-by: Torbjörn SVENSSON <torbjorn.svens...@foss.st.com>
>>>>> ---
>>>>> gcc/testsuite/gcc.target/arm/unsigned-extend-1.c | 1 +
>>>>> 1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/gcc/testsuite/gcc.target/arm/unsigned-extend-1.c b/gcc/
>>>>> testsuite/gcc.target/arm/unsigned-extend-1.c
>>>>> index 3b4ab048fb0..73f2e1a556d 100644
>>>>> --- a/gcc/testsuite/gcc.target/arm/unsigned-extend-1.c
>>>>> +++ b/gcc/testsuite/gcc.target/arm/unsigned-extend-1.c
>>>>> @@ -1,4 +1,5 @@
>>>>> /* { dg-do compile } */
>>>>> +/* { dg-require-effective-target arm_arm_ok } */
>>>>> /* { dg-options "-O2" } */
>>>>> unsigned char foo (unsigned char c)
>>>>
>>>
>>
>> I've just pushed a patch to eliminate one of the redundant zero-extends.
>> The other is harder to eliminate, so I've adjusted the test to xfail when
>> compiling for thumb1.
>
> Hi Richard,
>
> Can this be backported to releases/gcc-14 too?
> I can do the backport if you just approve it.
>
> Kind regards,
> Torbjörn
By all means backport the testsuite change, but not the change to thumb1.md,
please.
R.
