-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[flang] Re-enable setting noalias on procedure arguments.
#155949
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
Conversation
This is a follow-up on llvm#140803, which was disabled in llvm#142128 due to llvm#143219.
|
@llvm/pr-subscribers-flang-driver @llvm/pr-subscribers-flang-fir-hlfir Author: Slava Zakharin (vzakhari) ChangesThis is a follow-up on #140803, which was disabled in #142128 Full diff: https://github.com/llvm/llvm-project/pull/155949.diff 1 Files Affected:
diff --git a/flang/lib/Optimizer/Passes/Pipelines.cpp b/flang/lib/Optimizer/Passes/Pipelines.cpp
index 98f947a1f635d..6cc3290ff5cc2 100644
--- a/flang/lib/Optimizer/Passes/Pipelines.cpp
+++ b/flang/lib/Optimizer/Passes/Pipelines.cpp
@@ -14,7 +14,7 @@
/// Force setting the no-alias attribute on fuction arguments when possible.
static llvm::cl::opt<bool> forceNoAlias("force-no-alias", llvm::cl::Hidden,
- llvm::cl::init(false));
+ llvm::cl::init(true));
namespace fir {
|
|
Looks good to me. One suggestion: I don’t think the tests should be modified to use the flag to turn off noalias. It’s better to keep the tests using the default behavior, unless the test is specifically meant to verify the case where noalias should be avoided. |
tblah
left a comment
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.
LGTM.
nit: I agree the tests should be updated. I don't mind doing that for you in a separate PR if you don't have time. I understand the test conflicts are only because of my revert.
|
I am planning removing the Thank you for the review and all the work you put into trying to resolve the exchange2 issues! |
This is a follow up on llvm#155949. I removed -force-no-alias options from the tests and updated the checks.
This is a follow up on #155949. I removed -force-no-alias options from the tests and updated the checks.
This is a follow-up on #140803, which was disabled in #142128
due to #143219.