This is an archive of the discontinued LLVM Phabricator instance.

[DebugInfo] Remove the restriction on the size of DIStringType in DebugHandlerBase::isUnsignedDIType
ClosedPublic

Authored by cchen15 on Aug 23 2021, 8:18 AM.

Details

Summary

It turns out that the optimizations may correctly encode a Fortran character object as an integer even when the character length is greater than 1.

Also, it is probably not the AsmPrinter's job to catch the result of a bad transformation, which the removed assertion appears to be doing.

This is a follow-up to https://reviews.llvm.org/D100015.

Diff Detail

Event Timeline

cchen15 created this revision.Aug 23 2021, 8:18 AM
cchen15 requested review of this revision.Aug 23 2021, 8:18 AM
Herald added a project: Restricted Project. · View Herald Transcript
cchen15 retitled this revision from Remove the restriction on the size of DIStringType in DebugHandlerBase::isUnsignedDIType to [DebugInfo] Remove the restriction on the size of DIStringType in DebugHandlerBase::isUnsignedDIType.Aug 24 2021, 5:05 AM
aprantl accepted this revision.Aug 26 2021, 12:44 PM

Seems reasonable.

llvm/test/CodeGen/Generic/dbg-distringtype-uint.ll
68

Can you remove all attributes that are not strictly necessary (especially the ones in quotes)?

This revision is now accepted and ready to land.Aug 26 2021, 12:44 PM
cchen15 updated this revision to Diff 368969.Aug 26 2021, 1:21 PM
cchen15 added a project: debug-info.
cchen15 added inline comments.
llvm/test/CodeGen/Generic/dbg-distringtype-uint.ll
68

Excessive attributes eliminated. Thanks for the review, Adrian!

aprantl accepted this revision.Aug 27 2021, 12:36 PM

Hi @aprantl. I just got notified by buildbots that the new test fails on clang-aarch64-quick and clang-armv7-quick (maybe more to come). Could you please suggest a way to disable this test on triples that don't support the 'x86_64-unknown-linux-gnu' target? Thanks!

shchenz added a comment.EditedAug 30 2021, 6:01 PM

I think the reason is you specify triple for a case in target-independent dir(test/CodeGen/Generic), you need to add a "REQUIRES:" in the test file to make sure the test is only run on buildbots that supports X86-linux. See test/CodeGen/Generic/csw-debug-assert.ll.

I think the reason is you specify triple for a case in target-independent dir(test/CodeGen/Generic), you need to add a "REQUIRES:" in the test file to make sure the test is only run on buildbots that supports X86-linux. See test/CodeGen/Generic/csw-debug-assert.ll.

Even better is to move the test to the x86 subdirectory where the lit.cfg automatically adds this requirement to all tests.