-
Notifications
You must be signed in to change notification settings - Fork 16
[flang][fir] Add external name mangling pass #1061
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
8c3cd77
to
ca5d512
Compare
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.
Looks great to me. I wondering if this should be run by default by tco or not. Since tco is not a real Fortran driver, I guess it's OK to not run this pass by default.
Regarding the pass name, I am OK with it, the pedantic comment would be that --external-abi
might be a bit too generic since this is only dealing with the naming part of the ABI and that the target-rewrite pass (declared here) pass is also somehow Fortran ABI related. So --external-name-abi
might be more descriptive, but wait for @schweitzpgi takes on this before changing it.
ca5d512
to
f83e4ac
Compare
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.
Looks good. As far as what to call this, maybe something with "interoperability" and "name/naming" vs. "external ABI"? to focus on the naming aspect rather than anything about the binary interface.
f83e4ac
to
cf4fd39
Compare
Thanks for the review. I renamed most of the ABI into Name and the pass option is now |
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.
+1
The added pass works when mlir::LLVM::LLVMDialect, mlir::acc::OpenACCDialect, and mlir::omp::OpenMPDialect are present, which adds a dependency on those dialect libraries. https://github.com/flang-compiler/f18-llvm-project/blob/bb1ab876a5fec1eb3f2ab0603756f9124515a68a/flang/include/flang/Optimizer/Transforms/Passes.td#L189
The added pass works when mlir::LLVM::LLVMDialect, mlir::acc::OpenACCDialect, and mlir::omp::OpenMPDialect are present, which adds a dependency on those dialect libraries. https://github.com/flang-compiler/f18-llvm-project/blob/bb1ab876a5fec1eb3f2ab0603756f9124515a68a/flang/include/flang/Optimizer/Transforms/Passes.td#L189
The patch add a new pass called
ExternalNameConversion
. It converts the FIR internal name with the gfortran external name convention.The pass is not applied by default in
tco
.