Page MenuHomePhabricator

[SCEVExpander] Prefer pointer expansion for overflow checks
Needs ReviewPublic

Authored by reames on Jun 21 2021, 12:05 PM.

Details

Summary

We'd special cased this logic to use pointer types for non-integral pointers, but looking at the resulting code generation, the pointer version looks cleaner. We can simply drop the special casing, and use the pointer variant unconditionally.

Note that there are some test diffs, but a) running them through instcombine helps a ton, and b) there's enough missing obvious transforms on both before and after IR that it's clear this isn't performance sensitive.

This is mostly motivated by cleaning up mentions of non-integrals to have a clearer idea of what we actually need to support.

Diff Detail

Event Timeline

reames created this revision.Jun 21 2021, 12:05 PM
reames requested review of this revision.Jun 21 2021, 12:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 21 2021, 12:05 PM
reames updated this revision to Diff 353450.Jun 21 2021, 12:06 PM

Corrected patch.

Do we need to check that GEP index size is big enough for this?

Do we need to check that GEP index size is big enough for this?

I don't believe so, but maybe I'm missing something?

I don't think that increase of loop size may be justified by the fact that InstCombine will later clean it up. Loop passes may go in one loop pipeline, and passes like unswitch may be pessimized by this.

I don't think that increase of loop size may be justified by the fact that InstCombine will later clean it up. Loop passes may go in one loop pipeline, and passes like unswitch may be pessimized by this.

This comment doesn't make sense to me. What increase in loop size are you referring to? This change adjusts the type used for a generate test, nothing more.