This is an archive of the discontinued LLVM Phabricator instance.

[builtins] Do not assume sizeof(long long)==8 in int_div_impl.inc
AbandonedPublic

Authored by bjope on Apr 16 2020, 7:31 AM.

Details

Reviewers
MaskRay
Summary

Got some regressions downstream after commit b541196eb45d80f2d
due to the new code assuming sizeof(long long)==8. This is a fixup
for that problem.

And while being at it, also adding the missing file header.

Diff Detail

Event Timeline

bjope created this revision.Apr 16 2020, 7:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 16 2020, 7:31 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript
bjope updated this revision to Diff 258243.Apr 17 2020, 12:41 AM

Update after feedback (check sizeof(long long) rather than using CHAR_BIT).

bjope retitled this revision from [builtins] Do not assume CHAR_BIT==8 in int_div_impl.inc to [builtins] Do not assume sizeof(long long)==8 in int_div_impl.inc.Apr 17 2020, 12:42 AM
bjope edited the summary of this revision. (Show Details)
bjope updated this revision to Diff 258244.Apr 17 2020, 12:45 AM

Use "unsigned long long" instead of "long long" to match __builtin_clzll.

MaskRay added a comment.EditedApr 17 2020, 8:33 AM

Pushed 17772995d48b8c10a3142d602e228f3ebeed85bf instead

It is still unclear to me why assuming sizeof(long long)==8 can cause problems. You need to provide more information.

bjope added a comment.Apr 17 2020, 8:59 AM

Pushed 17772995d48b8c10a3142d602e228f3ebeed85bf instead

It is still unclear to me why assuming sizeof(long long)==8 can cause problems. You need to provide more information.

udivdi3 use du_int, which maps to unsigned long long. And with CHAR_BIT=16 we get sizeof(du_int)==sizeof(unsigned long lon)==4.
For udivdi3.c (which is doing typedef du_int fixuint_t) we ended up using
bultin_clz for the unsigned long long division, as the comparison was checking if sizeof(fixuint_t) was 8.