On 11/16/25 4:36 PM, H.J. Lu wrote:
On Mon, Nov 17, 2025 at 12:38 AM Jeff Law <[email protected]> wrote:



On 10/27/25 3:53 PM, H.J. Lu wrote:


As HJ indicated, I wouldn't expect significant fallout on load/store
architectures.  So the scope of fallout isn't likely to be too terrible.

I was going to throw it in my tester, but it doesn't apply cleanly on
the trunk.

patching file gcc/recog.cc
Hunk #1 FAILED at 116.
1 out of 2 hunks FAILED -- saving rejects to file gcc/recog.cc.rej


I put my branch at

https://gitlab.com/x86-gcc/gcc/-/commits/users/hjl/pr122343/master

on top of

commit 76943639ddd861dce3886d1def2a353ccfcdd585
Author: Eric Botcazou <[email protected]>
Date:   Mon Oct 27 19:51:11 2025 +0100

      Ada: Fix assertion failure on child generic package

Does it work for you?
So I generated a diff from that and put it into my tester with no
fallout.  But that's because it's disabled unless you have a suitable
hook defined for the target.

It would be helpful (and on the right track for integration) if the
branch just enabled it by default.


Here is the branch:

https://gitlab.com/x86-gcc/gcc/-/commits/users/hjl/pr122343/v3

to enable it by default.
So that's been in my tester for a few days with zero fallout. And AFAICT it is on by default (verified on the H8 which can do memory operations like addition/subtraction).

I don't have a high degree of confidence in coverage of volatile accesses in the testsuite. But I also don't have any concrete evidence of where this patch is likely to cause problems.

The latest version needs to be posted for review, but before you do that, I few comments on the version I tested from your branch.

I don't see any good reason to have a target hook to disable this behavior since we have a flag to control. If the patch truly preserves volatile semantics, then it should work everywhere and target disablement seems short-sighted.

Is it really necessary to keep that "try_combine_insn" object? It seems like you only use that to verify that the insn being recognized is a load, right? So if you supported stores you wouldn't really needed it, right?

Jeff

Reply via email to