This is an archive of the discontinued LLVM Phabricator instance.

[SCEV] Don't blindly transfer nowrap flags to pre-inc addrec
Needs ReviewPublic

Authored by nikic on Apr 21 2023, 8:59 AM.

Details

Summary

While writing a blog post I tried to explain why it is always safe to transfer IR nowrap flags to the pre-inc addrec, and ... couldn't, because I don't think it's correct. The most straightforward example would be two {0,+,1} addrecs, one of them has a nuw flag in IR but is never used, and the other doesn't have one and is used. We could incorrectly transfer the nuw flag from the former to the latter.

I believe we can transfer the nowrap flags only if either a) the addrec is known non-poison or b) the post-inc instruction being poison would cause UB in a latch-dominating instruction. In particular, this includes the IV being used in any exit condition.

This patch is still WIP because I need to properly update tests, but I wanted to put this up for some early feedback.

Diff Detail

Event Timeline

nikic created this revision.Apr 21 2023, 8:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 21 2023, 8:59 AM
nikic requested review of this revision.Apr 21 2023, 8:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 21 2023, 8:59 AM
mkazantsev added a comment.EditedApr 23 2023, 10:00 PM

I don't quite understand your example. Two AddRecs with same start/step bound to one loop with different flags simply cannot exist, it would be one entity in UniqueSCEVs. And if we were able to infer nuw it for it in some situation, it means we have proven that it does not overflow (meaning that the loop doesn't make more than UINT_MAX iterations) regardless of uses. I don't think flag inference should rely on users at all, because there is expander, and it could create new uses out of thin air.

llvm/test/Analysis/ScalarEvolution/different-loops-recs.ll
1

Precommit check regeneration?