Re: [PATCH v2] PR middle-end/123236: Simplify (int)((long long)x >> 4)
Jeffrey Law <[email protected]>
| Newsgroups | gmane.comp.gcc.patches |
|---|---|
| Message-ID | <[email protected]> |
On 8/11/2026 4:18 AM, Roger Sayle wrote:
> Hi Torbjorn,
> I believe this size regression on ARM (thumb) is resolved by the attached patch.
> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
> and make -k check, both with and without --target_board=unix{-m32}
> with no new failures. Ok for mainline?
>
> Hopefully, Jeff can spin this on his farm to see whether this transformation
> is still useful anywhere (and not just creating problems on ARM)?
>
> 2026-08-11 Roger Sayle <[email protected]>
>
> gcc/ChangeLog
> * combine.cc (simplify_comparison) <case AND>: Delete
> transformation that changed (AND (SUBREG x) C) into the
> non-canonical (SUBREG (AND (x C)).
Clean in all my tests so far. H8 is still running (it's particularly
long) and various emulated natives haven't tested, but the vast majority
of targets are done and clean.
So I did the archeological dig and briefly thought I may have added this
code. But it was just a merge from the old FSF GCC project into the
EGCS project.
The root of this code is a change from Richard K in 1998:
+ * combine.c (gen_binary): Don't make AND that does nothing.
+ (simplify_comparison, case AND): Commute AND and SUBREG.
+ * i386.h (CONST_CONSTS, case CONST_INT): One-byte integers are
cost 0.
We can make a reasonable guess that we had a useless AND and that
perhaps those cases could be triggered in a couple ways which this patch
works around.
In 1998 the GCC project wasn't great about including tests for most
changes so it's unlikely anything in the testsuite is going to break,
even if the useless AND was to return.
It's also the case that in 1998 we didn't have tree-ssa and the vast
majority of optimization was done in RTL, so a patch like this was
probably a lot more likely to trigger back then.
Anyway, the dig didn't give us anything conclusive other than the
original change wasn't to avoid a code correctness problem or an ICE.
It was much more likely to remove a useless instruction.
OK for the trunk with a testcase, which seems to be missing from the
patch AFAICT.
jeff
>
> Thanks in advance,
> Cheers,
> Roger
> --
>
>> -----Original Message-----
>> From: Torbjorn SVENSSON <[email protected]>
>> Sent: 10 August 2026 16:28
>> To: Richard Biener <[email protected]>; Roger Sayle
>> <[email protected]>; Richard Earnshaw (lists)
>> <[email protected]>
>> Cc: GCC Patches <[email protected]>; Jeffrey Law
>> <[email protected]>; Richard Sandiford
>> <[email protected]>
>> Subject: Re: [PATCH v2] PR middle-end/123236: Simplify (int)((long long)x >> 4)
>>
>> Hi,
>>
>> This change introduces a (size) regression for arm-none-eabi:
>>
>>
>> Testing arm/pr42879.c
>> doing compile
>> Executing on host: /build/r17-2833-g32657f29f91871/bin/arm-none-eabi-gcc
>> /build/gcc_src/gcc/testsuite/gcc.target/arm/pr42879.c -mthumb -
>> march=armv7-m -mcpu=cortex-m3 -mfloat-abi=soft -mfpu=auto -fdiagnostics-
>> plain-output -mthumb -Os -ffat-lto-objects -fno-ident -S -o pr42879.s
>> (timeout = 800)
>> spawn -ignore SIGHUP /build/r17-2833-g32657f29f91871/bin/arm-none-eabi-gcc
>> /build/gcc_src/gcc/testsuite/gcc.target/arm/pr42879.c -mthumb -march=armv7-
>> m -mcpu=cortex-m3 -mfloat-abi=soft -mfpu=auto -fdiagnostics-plain-output -
>> mthumb -Os -ffat-lto-objects -fno-ident -S -o pr42879.s pid is 180 -180 pid is -1
>> output is status 0
>>
>> PASS: gcc.target/arm/pr42879.c (test for excess errors)
>> FAIL: gcc.target/arm/pr42879.c scan-assembler lsls
>>
>>
>>
>> $ /build/r17-2412-g2fcbe075ec3b32/bin/arm-none-eabi-gcc
>> /build/gcc_src/gcc/testsuite/gcc.target/arm/pr42879.c -mthumb -march=armv7-
>> m -mcpu=cortex-m3 -mfloat-abi=soft -mfpu=auto -Os -fno-ident -S -o -
>> .cpu cortex-m3
>> .arch armv7-m
>> .fpu softvfp
>> .eabi_attribute 20, 1
>> .eabi_attribute 21, 1
>> .eabi_attribute 23, 3
>> .eabi_attribute 24, 1
>> .eabi_attribute 25, 1
>> .eabi_attribute 26, 1
>> .eabi_attribute 30, 4
>> .eabi_attribute 34, 1
>> .eabi_attribute 18, 4
>> .file "pr42879.c"
>> .text
>> .align 1
>> .global foo
>> .syntax unified
>> .thumb
>> .thumb_func
>> .type foo, %function
>> foo:
>> @ args = 0, pretend = 0, frame = 0
>> @ frame_needed = 0, uses_anonymous_args = 0
>> @ link register save eliminated.
>> ldrb r3, [r0] @ zero_extendqisi2
>> lsls r3, r3, #31
>> bmi .L2
>> b bar
>> .L2:
>> movs r0, #1
>> bx lr
>> .size foo, .-foo
>>
>>
>> $ /build/r17-2413-ge46d96d20bbfa7/bin/arm-none-eabi-gcc
>> /build/gcc_src/gcc/testsuite/gcc.target/arm/pr42879.c -mthumb -march=armv7-
>> m -mcpu=cortex-m3 -mfloat-abi=soft -mfpu=auto -Os -fno-ident -S -o -
>> .cpu cortex-m3
>> .arch armv7-m
>> .fpu softvfp
>> .eabi_attribute 20, 1
>> .eabi_attribute 21, 1
>> .eabi_attribute 23, 3
>> .eabi_attribute 24, 1
>> .eabi_attribute 25, 1
>> .eabi_attribute 26, 1
>> .eabi_attribute 30, 4
>> .eabi_attribute 34, 1
>> .eabi_attribute 18, 4
>> .file "pr42879.c"
>> .text
>> .align 1
>> .global foo
>> .syntax unified
>> .thumb
>> .thumb_func
>> .type foo, %function
>> foo:
>> @ args = 0, pretend = 0, frame = 0
>> @ frame_needed = 0, uses_anonymous_args = 0
>> @ link register save eliminated.
>> ldrb r3, [r0] @ zero_extendqisi2
>> and r3, r3, #1
>> cbnz r3, .L2
>> b bar
>> .L2:
>> movs r0, #1
>> bx lr
>> .size foo, .-foo
>>
>>
>>
>> Kind regards,
>> Torbjörn
>>
>> On 2026-07-14 11:07, Richard Biener wrote:
>>> On Fri, Jul 10, 2026 at 7:44 PM Roger Sayle <[email protected]>
>> wrote:
>>>>
>>>> Long story... Back in January I proposed the following patch:
>>>> https://gcc.gnu.org/pipermail/gcc-patches/2026-January/705292.html
>>>> which contained two pieces, one in combine, the other in simplify-rtx.
>>>> Both of which were initially approved by Jeff Law here
>>>> https://gcc.gnu.org/pipermail/gcc-patches/2026-May/715595.html
>>>> but then Richard Sandiford pointed out the combine piece might cause
>>>> problems on RISC machines, and suggested improvements.
>>>> https://gcc.gnu.org/pipermail/gcc-patches/2026-May/715668.html
>>>>
>>>> In addition to making Richard's recommended changes, I've also
>>>> decided to split the patch into two, to enable bisection and isolate
>>>> these transformations [in case Richard S's fears come to pass and the
>>>> combine transformation needs to be reverted].
>>>>
>>>> This is the "safe" (or less controversial) half. Hopefully, folks
>>>> are (still) happy for this bit to be committed?
>>>> p.s. the second and third hunks are just micro-optimizations; we
>>>> don't need to call simplify_gen_unary (TRUNCATE, ...) if the operand
>>>> already has the correct mode. The significant change is that the
>>>> modes don't need to match, and the operand need not be a register
>>>> [combine can put anything in a SUBREG].
>>>>
>>>> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
>>>> and make -k check, both with and without --target_board=unix{-m32},
>>>> with no new failures. Ok for mainline?
>>> OK.
>>>
>>> Thanks,
>>> Richard.
>>>
>>>> 2026-07-10 Roger Sayle <[email protected]>
>>>>
>>>> gcc/ChangeLog
>>>> PR rtl-optimization/123236
>>>> * simplify-rtx.cc (simplify_context::simplify_truncation): Handle
>>>> cases where a ZERO_EXTRACT or SIGN_EXTRACT has a different mode
>>>> to (but at least as wide as) its first operand.
>>>>
>>>> gcc/testsuite/ChangeLog
>>>> PR rtl-optimization/123236
>>>> * gcc.target/i386/pr123236-1.c: New test case.
>>>>
>>>>
>>>> Thank again,
>>>> Roger
>>>> --
>>>>