-
Notifications
You must be signed in to change notification settings - Fork 479
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.
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
Add a compiler flag for the evaluateBuiltins pass #6450
Conversation
@@ -171,10 +171,12 @@ pluginOptions = | |||
, Implication (== True) posPreserveLogging True | |||
, Implication (== True) posCaseOfCaseConservative True | |||
, Implication (== True) posInlineConstants False | |||
, Implication (== True) posDoSimplifierEvaluateBuiltins False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It gets turned off here because it may increase program size?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it may change semantics: #6167
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late review.
, Implication (== False) posRelaxedFloatin True | ||
, Implication (== False) posPreserveLogging False | ||
, Implication (== False) posCaseOfCaseConservative False | ||
, Implication (== False) posInlineConstants True | ||
, Implication (== False) posDoSimplifierEvaluateBuiltins True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My OCD would prefer id
over (== True)
and not
over (== False)
, but I guess it's currently more readable.
@@ -230,6 +232,11 @@ pluginOptions = | |||
, let k = "simplifier-beta" | |||
desc = "Run a simplification pass that performs beta transformations" | |||
in (k, PluginOption typeRep (setTrue k) posDoSimplifierBeta desc []) | |||
, let k = "simplifier-evaluate-builtins" | |||
desc = | |||
"Run a simplification pass that evaluates fully saturated builtin applications. \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add some comments! To the pass itself and somewhere here, so that the user knows they are enabling a semantics-changing pass. At least the link to the issue.
This flag is currently missing,
_posDoSimplifierEvaluateBuiltins
is always true and cannot be turned off.