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.
R.
