https://gcc.gnu.org/bugzilla/show_bug.cgi?id=50955

amker at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |amker at gcc dot gnu.org

--- Comment #21 from amker at gcc dot gnu.org ---
For the record, here are some findings for this issue.

1) It isn't well-defined in terms of C standard to do below transformation in
GCC:
     a) convert pointers to unsigned integers;
     b) do arithmetic computations on converted unsigned integers;
        like: (szietype)ptr1 - (sizetype)ptr2 - 1 + (sizetype)ptr2
     c) convert result integer back to pointer;
     d) access memory using the result pointer;

2) GCC may still generates such transformation.  As said it's too conservative
to follow C standard strictly in this aspect.

3) When doing such transformation, we need to guarantee TMR.base is derived
correctly.  Otherwise GCC's alias analysis would be confused and results in
wrong code.

4) The problem with IVOPT is the approach it takes to derive TMR.base. 
Firstly, it depends on iv_cand.base_object to derive correct base for TMR. 
This is why GCC skips candidate with base_object for any iv_use with different
base_object.

5) when approach in 4) fails, it falls back to primitive approach by simply
setting TMR.base to any pointer in the result address expression.  Of course,
it could be wrong.  This is why PR50955 is triggered in the first place.

With these understandings, I can see two possible fixes here.

Fix 1) Just like Richard has committed, we not only check different base_object
on address type iv_uses, but also on generic type iv_uses.  This is too
conservative and we need to resolve performance regressions reported in
PR52272.

Fix 2) We need to improve how TMR.base is derived in IVOPTs.  New approach is
instead of iv_cand.base_object, we can use iv_use.base_object.  That is, check
the result address expression to see if it includes iv_use.base_object.  If
yes, we can set TMR.base to it; otherwise we can reject the iv_cand.  (actually
we can create a temp ssa_var with iv_use.base_object's pointer information
copied)

There is one concern about Fix 2).  The get_computation_cost_at needs to be
re-implemented to compute the result address expression like
get_computation_aff does.

Also Fix 2) doesn't give more benefit comparing to proposal patch in PR52272. 
Though that proposal has its own problem.

Reply via email to