Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[Codegen][LLVM] Add ability to turn on fast math flags #9223
[Codegen][LLVM] Add ability to turn on fast math flags #9223
Changes from 14 commits
18ca603
1c8da0f
444e51a
6938d70
17fa49c
2795510
02cd251
3d6c2c3
b244dec
0c5d38b
c9ac146
99ae59f
d9e3524
5466663
cfeb699
2abbed5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
A related comment: This code path should hit by default, otherwise this change would introduce regression.
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.
Hmm, I see fair enough, I think OptLevel 3 should not be the default but let me get some data to support this first. Changed to make OptLevel 3 the default.
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.
I think we just inherited opt level being 3 by default from Halide: https://github.com/halide/Halide/blob/ed87acb466f13144be235a33a30f242e30a6a74f/src/CodeGen_LLVM.cpp#L1145
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.
See
tvm/src/target/llvm/codegen_llvm.cc
Line 346 in 3229cb3
I don't see why users would want to choose an opt level other than 3. However, internally we may want to prefer faster compile time for the constant folding use case (which currently compiles every subgraph with opt level = 3).
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.
Hmm good catch. Need to see what the difference between the two optimization settings is.
As for the right Opt-Level, I think 3 can lead to slow downs in some situations (granted this is about gcc but same idea):
https://wiki.gentoo.org/wiki/GCC_optimization#-O
-O3: the highest level of optimization possible. It enables optimizations that are expensive in terms of compile time and memory usage. Compiling with -O3 is not a guaranteed way to improve performance, and in fact, in many cases, can slow down a system due to larger binaries and increased memory usage. -O3 is also known to break several packages. Using -O3 is not recommended. However, it also enables -ftree-vectorize so that loops in the code get vectorized and will use AVX YMM registers.
I did run a trial of fast math + changes the TargetMachine opt level to O2 and some models were faster and some were slower.
So we should add the flag to make it easy to test.
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.
Hmm so the link listed is for the PassManager. Higher opt level = more passes much like we have relay opt level. It appears to be associated with -O3 in clang. However, CodeGenOpts appears to be a separate thing that is set in clang too https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/BackendUtil.cpp#L935. Looks like this emits the assembly. This is also associated with clang's -O3.
So the flag should control everything.
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.
I made it based on the -O flag added. Default is still 2 however (which is the default for clang I believe)