This is an archive of the discontinued LLVM Phabricator instance.

[IR] Reduce max supported integer from 2^24-1 to 2^23.
ClosedPublic

Authored by craig.topper on Sep 13 2021, 1:50 PM.

Details

Summary

SelectionDAG will promote illegal types up to a power of 2 before
splitting down to a legal type. This will create an IntegerType
with a bit width that must be <= MAX_INT_BITS. This places an
effective upper limit on any type of 2^23 so that we don't try
create a 2^24 type.

I considered putting a fatal error somewhere in the path from
TargetLowering::getTypeConversion down to IntegerType::get, but
limiting the type in IR seemed better.

This breaks backwards compatibility with IR that is using a really
large type. I suspect such IR is going to be very rare due to the
the compile time costs such a type likely incurs.

Prevents the ICE in PR51829.

Diff Detail

Event Timeline

craig.topper created this revision.Sep 13 2021, 1:50 PM
craig.topper requested review of this revision.Sep 13 2021, 1:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 13 2021, 1:50 PM

LGTM.

I agree the largest allowed integer type should be a power of two, and that there isn't any practical backwards-compatibility issue.

efriedma accepted this revision.Sep 13 2021, 3:07 PM
This revision is now accepted and ready to land.Sep 13 2021, 3:07 PM
rnk requested changes to this revision.Sep 13 2021, 3:12 PM

Sounds good, but I think the clang presubmit test failures (ext-int.c[pp]) are real failures that need to be addressed before landing.

This revision now requires changes to proceed.Sep 13 2021, 3:12 PM

Sounds good, but I think the clang presubmit test failures (ext-int.c[pp]) are real failures that need to be addressed before landing.

I had addressed those with D109714 which I failed to link here. I wrote that thinking I would make a SelectionDAG specific fix, but now that we're changing the constant I can drop the Sema change from D109714 and merge the patches.

Update clang tests.

Herald added a project: Restricted Project. · View Herald TranscriptSep 13 2021, 4:23 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
This revision was not accepted when it landed; it landed in state Needs Review.Sep 14 2021, 8:12 AM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.