On Mon, 1 Jun 2020, Feng Xue OS via Gcc-patches wrote:
* match.pd ((PTR + A) - (PTR + B)) -> (ptrdiff_t)(A - B): New
simplification.
Not new, modified.
* ((PTR_A + O) - (PTR_B + O)) -> (PTR_A - PTR_B): New simplification.
O might not be the best choice because of how close it looks to 0.
- (simplify
- (pointer_diff (pointer_plus @@0 @1) (pointer_plus @0 @2))
- /* The second argument of pointer_plus must be interpreted as signed, and
- thus sign-extended if necessary. */
- (with { tree stype = signed_type_for (TREE_TYPE (@1)); }
- /* Use view_convert instead of convert here, as POINTER_PLUS_EXPR
- second arg is unsigned even when we need to consider it as signed,
- we don't want to diagnose overflow here. */
- (minus (convert (view_convert:stype @1))
- (convert (view_convert:stype @2)))))))
+ (simplify
+ (pointer_diff (pointer_plus@3 @0 @1) (pointer_plus @0 @2))
+ (if (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@3)))
+ (convert (minus @1 @2))))
What don't you like about the existing transformation? You are replacing a
transformation that always folds by one that folds only in some cases, and
looses the information that some overflows cannot happen. That looks like
it is making things worse from an optimization point of view. Do you
consider the transformation as unsafe with -fsanitize=pointer-overflow
(does that correspond to the case where TYPE_OVERFLOW_UNDEFINED is true
for a pointer type?)?
Ah, looking at the PR, you decided to perform the operation as unsigned
because that has fewer NOP conversions, which, in that particular testcase
where the offsets are originally unsigned, means we simplify better. But I
would expect it to regress other testcases (in particular if the offsets
were originally signed). Also, changing the second argument of
pointer_plus to be signed, as is supposed to eventually happen, would
break your testcase again.
At the very least we want to keep a comment next to the transformation
explaining the situation.
If there are platforms where the second argument of pointer_plus is a
smaller type than the result of pointer_diff (can this happen? I keep
forgetting all the weird things some platforms do), this version may do an
unsafe zero-extension.
I think I would rather try to extend the transformation for A*C-B*C.
+ (simplify
+ (pointer_diff (pointer_plus@3 @0 @2) (pointer_plus @1 @2))
+ (if (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@3))
+ && !integer_zerop (@2))
+ (pointer_diff @0 @1)))))
Why would it be a problem if @2 is 0? We should have already simplified
(pointer_plus @i integer_zerop) to just @i before reaching this, but it
doesn't make the transformation wrong.
Same remark as above for TYPE_OVERFLOW_UNDEFINED, is that for the sake of
-fsanitize=pointer-overflow?
--
Marc Glisse
