Skip to content

gh-101282: Group and quote BOLT CLI flags#104821

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
erlend-aasland wants to merge 1 commit into from
Closed

gh-101282: Group and quote BOLT CLI flags #104821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion configure
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

43 changes: 26 additions & 17 deletions configure.ac
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2085,25 +2085,34 @@ AC_ARG_VAR(
AC_MSG_CHECKING([BOLT_APPLY_FLAGS])
if test -z "${BOLT_APPLY_FLAGS}"
then
AS_VAR_SET(
[BOLT_GENERIC_OPTIONS],
[m4_normalize(["
-dyno-stats
-update-debug-sections
-use-gnu-stack
"])]
)
AS_VAR_SET(
[BOLT_OPTIMIZATION_OPTIONS],
[m4_normalize(["
-frame-opt=hot
-icf=1
-indirect-call-promotion=all
-inline-all
-inline-ap
-jump-tables=aggressive
-peepholes=none
-reorder-blocks=ext-tsp
-reorder-functions-use-hot-size
-reorder-functions=hfsort+
-split-eh
-split-functions
"])]
)
AS_VAR_SET(
[BOLT_APPLY_FLAGS],
[m4_join([ ],
[-update-debug-sections],
[-reorder-blocks=ext-tsp],
[-reorder-functions=hfsort+],
[-split-functions],
[-icf=1],
[-inline-all],
[-split-eh],
[-reorder-functions-use-hot-size],
[-peepholes=none],
[-jump-tables=aggressive],
[-inline-ap],
[-indirect-call-promotion=all],
[-dyno-stats],
[-use-gnu-stack],
[-frame-opt=hot]
)]
["${BOLT_GENERIC_OPTIONS} ${BOLT_OPTIMIZATION_OPTIONS}"]
Copy link
Member

@corona10corona10May 23, 2023•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One pain point of this change is we have to care double quote in multiple places.
(defining BOLT_GENERIC_OPTIONS, defining BOLT_OPTIMIZATION_OPTIONS and BOLT_GENERIC_OPTIONS + BOLT_OPTIMIZATION_OPTIONS)

Copy link
ContributorAuthor

@erlend-aaslanderlend-aaslandMay 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree; I don't see that as a "pain point".

Copy link
Member

@corona10corona10May 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree; I don't see that as a "pain point".

Well, My preference is don't care about double quotes as possible.
(This was the root cause of this issue in my guess)
I feel quite uncomfortable maintaining three elements that should be noted by double quotes.
It could be my personal preference. but yeah that's why I said pain point.

)
fi
AC_MSG_RESULT([$BOLT_APPLY_FLAGS])
Expand Down