This is an archive of the discontinued LLVM Phabricator instance.

[Loads] Return false in canReplacePointersIfEqual helper for non-aliasing pointers
AbandonedPublic

Authored by mnadeem on Feb 1 2023, 4:06 PM.

Diff Detail

Event Timeline

mnadeem created this revision.Feb 1 2023, 4:06 PM
mnadeem requested review of this revision.Feb 1 2023, 4:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 1 2023, 4:06 PM
aqjune accepted this revision.Feb 1 2023, 4:23 PM

Looks good to me in high level, but I suggest waiting for one more accept since I haven't been working with the LLVM codebase for a while.

This revision is now accepted and ready to land.Feb 1 2023, 4:23 PM
nikic requested changes to this revision.Feb 2 2023, 12:41 AM
nikic added a subscriber: nikic.

I don't think this is the correct check to do, and I also don't particularly want to introduce AA queries for this. The correct check is whether getUnderlyingObject(A) == getUnderlyingObject(B), in reasonable approximation. (There are more cases that are safe, e.g. a null pointer can be replaced with anything, but they are probably not very useful.)

I believe that if we actually want to fix this class of problems (which goes beyond GVN), we should fix it properly, and that starts with using the right predicate so we get a correct idea of the performance regressions we should expect.

This revision now requires changes to proceed.Feb 2 2023, 12:41 AM
mnadeem abandoned this revision.Feb 27 2023, 11:56 AM

Ported all changes to the patch I previously added as the child revision.