On 5/21/25 4:29 PM, Andrew Pinski wrote:
On Wed, May 21, 2025 at 3:21 PM Jeff Law <jeffreya...@gmail.com> wrote:



On 5/5/25 3:27 PM, Andrew Pinski wrote:
The problem here is simplify-rtx.cc expects gen_lowpart_no_emit
to return NULL on failure but combine's hook was returning CLOBBER.
After r16-160-ge6f89d78c1a7528e93458278, gcc.target/i386/avx512bw-pr103750-2.c
started to fail at -m32 due to this as new simplify code would return
a RTL with a clobber in it rather than returning NULL.
To fix this gen_lowpart_no_emit should return NULL when there was an failure
instead of a clobber. This only changes the gen_lowpart_no_emit hook and not the
generic gen_lowpart hook as parts of combine just pass gen_lowpart result 
directly
without checking the return value.

Bootstrapped and tested on x86_64-linux-gnu.

       PR rtl-optimization/120090
gcc/ChangeLog:

       * combine.cc (gen_lowpart_for_combine_no_emit): New function.
       (RTL_HOOKS_GEN_LOWPART_NO_EMIT): Set to gen_lowpart_for_combine_no_emit.
So that's standard combine behavior from as long as I can remember.  So
I'm naturally worried about the various chunks of combine that are
expecting to see valid RTL objects (the CLOBBER) now getting pass a
NULL.  If we substitute that value into an insn and try to recognize it
we could well run into a ICE or segfault.

Yes I had worry too which is why I didn't change
gen_lowpart_for_combine. and only changed the no_emit hook.
Combine itself does not call the no_emit hook directly but rather than
the full gen_lowpart which then calls the non no_emit hook version.
And it is why I wrote:
```
This only changes the gen_lowpart_no_emit hook and not the
generic gen_lowpart hook as parts of combine just pass gen_lowpart
result directly
without checking the return value.
```
In my original commit message of why we can't just change gen_lowpart hook.
I clearly missed the significance of that comment. THanks for clarifying. Ok for the trunk.

jeff

Reply via email to