This is an archive of the discontinued LLVM Phabricator instance.

[Flang][Driver] Add regex support for R_Group options
ClosedPublic

Authored by victorkingi on Aug 21 2023, 9:22 AM.

Details

Summary

Add regex handling for all variations of OPT_R_Joined, i.e.
-Rpass, -Rpass-analysis, -Rpass-missed.

Depends on D158174. That patch implements backend support for
R_Group options.

Diff Detail

Event Timeline

victorkingi created this revision.Aug 21 2023, 9:22 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
victorkingi requested review of this revision.Aug 21 2023, 9:22 AM
victorkingi added inline comments.Aug 22 2023, 1:51 AM
flang/lib/Frontend/CompilerInvocation.cpp
766–768

For the bad regex error to be printed with colors, we would need to pass showColors from Frontend Options to Diagnostics Options much earlier on in CompilerInvocation::createFromArgs . Reason being regex errors from R family of flags don't get that far into the execution.

awarzynski added inline comments.Aug 27 2023, 7:37 AM
flang/lib/Frontend/CompilerInvocation.cpp
164

[nit] This is unrelated to this change, so just a nice-to-have

256

Just a nice-to-have

1035–1036

So, in -Rpass=<regex>you would only push -Rpass, right?

flang/test/Driver/optimization-remark.f90
18

At the moment this test does not differentiate between -Rpass=loop and -Rpass - in both cases the output is identical. Could you extend the test so that there's an actual difference?

victorkingi marked 3 inline comments as done.

Changed optimization-remark.f90 test code to generate more unique
missed and successful passes. -Rpass=loop and -Rpass should have
unique outputs hence, testable.
Minor comments added in CompilerInvocation.cpp

victorkingi marked an inline comment as done.Aug 29 2023, 6:23 AM
victorkingi added inline comments.
flang/lib/Frontend/CompilerInvocation.cpp
1035–1036

You will only push "pass". I've updated the comment to make it clearer.

Moved invalid regex test to optimization-remark-invalid.f90

Makes sense, a few minor suggestion. Also, could you rebase on top of main?

flang/test/Driver/optimization-remark.f90
15

[nit] IMHO, PASS-LOOP-ONLY would be more descriptive. Or even PASS-REGEX-LOOP-ONLY. "REGEX" refers to an implementation detail, but here we are more interested in the actual functionality.

35

Some comments explaining _what to expect_ would help. For example, with plain -Rpass we expect remarks related to 2 opportunities. Once we start filtering, this is reduced to 1, right?

39

Could you re-order the CHECK lines so that it matches the RUN lines? So:

! REMARKS:
! NO-REMARKS:

! PASS-REGEX-NOT:
! PASS-REGEX:

! MISSED-REGEX-NOT:
! MISSED-REGEX:

! ANALYSIS-REGEX-NOT:
! ANALYSIS-REGEX:
victorkingi marked 3 inline comments as done.

Reused same definition of the output in optimization-remark.f90 as well
as discarded the output generated.

Replaced '-c' flag references with '-S' to only handle pre-process and
compile steps in optimization-remark.f90.

flang/test/Driver/optimization-remark.f90
35

Yes

awarzynski accepted this revision.Aug 30 2023, 6:43 AM

LGTM, thank you for addressing my comments!

flang/test/Driver/optimization-remark.f90
72–78
This revision is now accepted and ready to land.Aug 30 2023, 6:43 AM
victorkingi marked an inline comment as done.

updated comment in optimization-remark.f90, attempt to fix failing test
case.

Attempt to fix failing optimization-remark.f90 test

[nit] optimization-remark.f90 line 80 'enddo' to 'end do'

no changes, rebasing

no changes, rebasing

This revision was landed with ongoing or failed builds.Aug 31 2023, 4:16 AM
This revision was automatically updated to reflect the committed changes.