On 2/1/24 07:20, Richard Biener wrote:
The following avoids re-associating

  (minus:DI (reg/f:DI 119)
     (minus:DI (reg/f:DI 120)
         (reg/f:DI 114)))

into

  (minus:DI (plus:DI (reg/f:DI 114)
         (reg/f:DI 119))
     (reg/f:DI 120))

as that possibly confuses the REG_POINTER heuristics of RTL
alias analysis.  This happens to miscompile the PRs testcase
during DSE which expands addresses via CSELIB which eventually
simplifies what it substituted to.  The original code does
the innocent ptr - (ptr2 - ptr2'), bias a pointer by the
difference of two other pointers.

--

This is what I propose for the PR for branches, I have not made much
progress with fixing the fallout on the RTL alias analysis change
on trunk, so this is the alternative if we decide to revert that.

Bootstrapped and tested on x86_64-unknown-linux-gnu on the gcc-13
branch, bootstrapped after reverting of the previous fix on
x86_64-unknown-linux-gnu on trunk, testing is still ongoing there.

OK?  Any preference for trunk?

Thanks,
Richard.

        PR rtl-optimization/113255
        * simplify-rtx.cc (simplify_context::simplify_binary_operation_1):
        Do not re-associate a MINUS with a REG_POINTER op0.
Nasty little set of problems. I don't think we ever pondered that we could have multiple REGNO_POINTER_FLAG objects in the same expression, but clearly that can happen once you introduce a 3rd term in the expression.

I don't mind avoiding the reassociation, but it feels like we're papering over problems in alias.cc. Conceptually it seems like if we have two objects with REG_POINTER set, then we can't know which one is the real base. So your patch in the PR wasn't that bad.

Alternately, just stop using REG_POINTER for alias analysis? It looks fundamentally flawed to me in that context. In fact, one might argue that the only legitimate use would be to indicate to the target that we know a pointer points into an object. Some targets (the PA) need this because x + y is not the same as y + x when used as a memory address.

If we wanted to be a bit more surgical, drop REG_POINTER from just the MINUS handling in alias.cc?

Jeff

Reply via email to