This is an archive of the discontinued LLVM Phabricator instance.

[Clang] Improve diagnostic message for loop hint pragma
ClosedPublic

Authored by eopXD on Oct 26 2022, 1:08 PM.

Details

Summary

Originally the loop hint is not displayed correctly in the diagnostic.
This patch fixes it.

Diff Detail

Event Timeline

eopXD created this revision.Oct 26 2022, 1:08 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 26 2022, 1:08 PM
eopXD requested review of this revision.Oct 26 2022, 1:08 PM
eopXD updated this revision to Diff 470913.Oct 26 2022, 1:12 PM

Trigger upon parent revision.

eopXD retitled this revision from [Clang] Improve diagnotisc message for loop hint pragma to [Clang] Improve diagnostic message for loop hint pragma.Oct 26 2022, 1:13 PM
SjoerdMeijer added inline comments.
clang/lib/Parse/ParsePragma.cpp
1306

We've got some duplication now, as "loop" is also checked in line 1310. Can you see if you can merge these checks, simplify things here a bit?

eopXD updated this revision to Diff 471048.Oct 27 2022, 1:06 AM

Rebase upon latest main and simplify existing checks.

eopXD marked an inline comment as done.Oct 27 2022, 1:07 AM
eopXD added inline comments.
clang/lib/Parse/ParsePragma.cpp
1306

Yes, the case for loop in below can now be removed. Thank you.

SjoerdMeijer added inline comments.Oct 27 2022, 1:22 AM
clang/lib/Parse/ParsePragma.cpp
1306

I was hoping that modifying the "loop" case would work:

.Case("loop", (llvm::Twine("clang loop ") + Option.getIdentifierInfo()->getName()).str());
eopXD updated this revision to Diff 471057.Oct 27 2022, 1:27 AM
eopXD marked an inline comment as done.

Update code based on suggestion.

eopXD marked an inline comment as done.Oct 27 2022, 1:28 AM
eopXD added inline comments.
clang/lib/Parse/ParsePragma.cpp
1306

Your suggestion is better than mine, thank you :)

eopXD marked an inline comment as done.Oct 27 2022, 2:05 AM
Meinersbur accepted this revision.Oct 27 2022, 1:17 PM

LGTM

clang/lib/Parse/ParsePragma.cpp
1308

ClangLoopStr is unused now

This revision is now accepted and ready to land.Oct 27 2022, 1:17 PM
eopXD updated this revision to Diff 471344.Oct 27 2022, 6:03 PM

Update code to handle when IdentifierInfo is null.
The .Case syntax seems to evaulate expression in other non-trigger cases.
The "loop" case was triggered for test case in loop unroll and jam, resulting
in segmentation fault. Anyway the updated code shall be safe now.

This revision was automatically updated to reflect the committed changes.