You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/10/07 22:11:25 UTC

[GitHub] [tvm] AndrewZhaoLuo opened a new pull request #9223: [WIP][Codegen][LLVM] Add ability to turn on fast math flags

AndrewZhaoLuo opened a new pull request #9223:
URL: https://github.com/apache/tvm/pull/9223


   Got to benchmark this so until then WIP 
   
   Test code: 
   
   See https://github.com/AndrewZhaoLuo/TVM-Sandbox/blob/fd08f88c12c9562a0e0f72dd7ff60f398452de35/codegen/test_export_to_ll.py#L8
   
   By setting the environment flag the generated LLVM ASM code is different:
   
   Without fastmath:
   ```
   ; Function Attrs: nofree noinline norecurse nounwind
   define internal fastcc void @test_add_compute_(i8* noalias nocapture align 128 %0, i8* noalias nocapture readonly align 128 %1, i8* noalias nocapture readonly align 128 %2) unnamed_addr #1 {
   entry:
     %3 = bitcast i8* %1 to <2 x float>*
     %4 = load <2 x float>, <2 x float>* %3, align 128, !tbaa !114
     %5 = bitcast i8* %2 to <2 x float>*
     %6 = load <2 x float>, <2 x float>* %5, align 128, !tbaa !117
     %7 = fadd <2 x float> %4, %6
     %8 = bitcast i8* %0 to <2 x float>*
     store <2 x float> %7, <2 x float>* %8, align 128, !tbaa !120
     ret void
   }
   ```
   
   With fastmath:
   ```
   ; Function Attrs: nofree noinline norecurse nounwind
   define internal fastcc void @test_add_compute_(i8* noalias nocapture align 128 %0, i8* noalias nocapture readonly align 128 %1, i8* noalias nocapture readonly align 128 %2) unnamed_addr #1 {
   entry:
     %3 = bitcast i8* %1 to <2 x float>*
     %4 = load <2 x float>, <2 x float>* %3, align 128, !tbaa !114
     %5 = bitcast i8* %2 to <2 x float>*
     %6 = load <2 x float>, <2 x float>* %5, align 128, !tbaa !117
     %7 = fadd fast <2 x float> %6, %4
     %8 = bitcast i8* %0 to <2 x float>*
     store <2 x float> %7, <2 x float>* %8, align 128, !tbaa !120
     ret void
   }
   ```
   
   Note the `fast` tag to the `fadd` operations now.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] AndrewZhaoLuo commented on a change in pull request #9223: [Codegen][LLVM] Add ability to turn on fast math flags

Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on a change in pull request #9223:
URL: https://github.com/apache/tvm/pull/9223#discussion_r731344719



##########
File path: src/target/llvm/codegen_llvm.cc
##########
@@ -343,7 +345,26 @@ void CodeGenLLVM::Optimize() {
 
   // place optimization pass
   llvm::PassManagerBuilder builder;
-  builder.OptLevel = 3;
+
+  // Use the same opt-level as specified in TargetMachine for running passes
+  llvm::CodeGenOpt::Level opt_level = target_machine_->getOptLevel();
+
+  switch (opt_level) {
+    case llvm::CodeGenOpt::Level::None:
+      builder.OptLevel = 0;
+      break;
+    case llvm::CodeGenOpt::Level::Less:
+      builder.OptLevel = 1;
+      break;
+
+    case llvm::CodeGenOpt::Level::Default:
+      builder.OptLevel = 2;
+      break;
+
+    default:
+      // CodeGenOpt::Level::Aggressive
+      builder.OptLevel = 3;

Review comment:
       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.

##########
File path: src/target/llvm/codegen_llvm.cc
##########
@@ -343,7 +345,26 @@ void CodeGenLLVM::Optimize() {
 
   // place optimization pass
   llvm::PassManagerBuilder builder;
-  builder.OptLevel = 3;
+
+  // Use the same opt-level as specified in TargetMachine for running passes
+  llvm::CodeGenOpt::Level opt_level = target_machine_->getOptLevel();
+
+  switch (opt_level) {
+    case llvm::CodeGenOpt::Level::None:
+      builder.OptLevel = 0;
+      break;
+    case llvm::CodeGenOpt::Level::Less:
+      builder.OptLevel = 1;
+      break;
+
+    case llvm::CodeGenOpt::Level::Default:
+      builder.OptLevel = 2;

Review comment:
       Changed




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] AndrewZhaoLuo commented on a change in pull request #9223: [Codegen][LLVM] Add ability to turn on fast math flags

Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on a change in pull request #9223:
URL: https://github.com/apache/tvm/pull/9223#discussion_r731310152



##########
File path: src/target/llvm/llvm_common.cc
##########
@@ -139,8 +152,22 @@ std::unique_ptr<llvm::TargetMachine> GetLLVMTargetMachine(const Target& target,
     ICHECK(allow_null) << err << " target_triple=" << target_triple;
     return nullptr;
   }
-  llvm::TargetMachine* tm =
-      llvm_target->createTargetMachine(target_triple, mcpu, mattr, opt, llvm::Reloc::PIC_);
+
+  Integer llvm_opt_level = target->GetAttr<Integer>("O").value_or(Integer(2));

Review comment:
       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.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] masahi merged pull request #9223: [Codegen][LLVM] Add ability to turn on fast math flags

Posted by GitBox <gi...@apache.org>.
masahi merged pull request #9223:
URL: https://github.com/apache/tvm/pull/9223


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] masahi commented on pull request #9223: [WIP][Codegen][LLVM] Add ability to turn on fast math flags

Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #9223:
URL: https://github.com/apache/tvm/pull/9223#issuecomment-938496615


   I think it is better to make use of a target attribute, e.g. `target = "llvm -fastmath"`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] AndrewZhaoLuo commented on pull request #9223: [Codegen][LLVM] Add ability to turn on fast math flags

Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on pull request #9223:
URL: https://github.com/apache/tvm/pull/9223#issuecomment-941702025


   This is ready for review now


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] masahi commented on a change in pull request #9223: [Codegen][LLVM] Add ability to turn on fast math flags

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #9223:
URL: https://github.com/apache/tvm/pull/9223#discussion_r731342213



##########
File path: src/target/llvm/codegen_llvm.cc
##########
@@ -343,7 +345,26 @@ void CodeGenLLVM::Optimize() {
 
   // place optimization pass
   llvm::PassManagerBuilder builder;
-  builder.OptLevel = 3;
+
+  // Use the same opt-level as specified in TargetMachine for running passes
+  llvm::CodeGenOpt::Level opt_level = target_machine_->getOptLevel();
+
+  switch (opt_level) {
+    case llvm::CodeGenOpt::Level::None:
+      builder.OptLevel = 0;
+      break;
+    case llvm::CodeGenOpt::Level::Less:
+      builder.OptLevel = 1;
+      break;
+
+    case llvm::CodeGenOpt::Level::Default:
+      builder.OptLevel = 2;

Review comment:
       Since you are using `target->GetAttr<Integer>("O").value_or(Integer(2));` in `llvm_common.cc`, do we always hit this code path if users do not specify the opt level? 
   
   Previously we were always using `builder.OptLevel = 3;`, so the default here shouldn't change. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] masahi commented on a change in pull request #9223: [Codegen][LLVM] Add ability to turn on fast math flags

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #9223:
URL: https://github.com/apache/tvm/pull/9223#discussion_r728534141



##########
File path: src/target/llvm/llvm_common.cc
##########
@@ -139,8 +152,22 @@ std::unique_ptr<llvm::TargetMachine> GetLLVMTargetMachine(const Target& target,
     ICHECK(allow_null) << err << " target_triple=" << target_triple;
     return nullptr;
   }
-  llvm::TargetMachine* tm =
-      llvm_target->createTargetMachine(target_triple, mcpu, mattr, opt, llvm::Reloc::PIC_);
+
+  Integer llvm_opt_level = target->GetAttr<Integer>("O").value_or(Integer(2));

Review comment:
       See https://github.com/apache/tvm/blob/3229cb329254764499dd672bb28fd9685ecd6a2e/src/target/llvm/codegen_llvm.cc#L346. I think they refer to the same opt level.
   
   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).




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] masahi commented on pull request #9223: [Codegen][LLVM] Add ability to turn on fast math flags

Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #9223:
URL: https://github.com/apache/tvm/pull/9223#issuecomment-946202046


   cc @kparzysz-quic @junrushao1994 @tqchen 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] AndrewZhaoLuo commented on a change in pull request #9223: [Codegen][LLVM] Add ability to turn on fast math flags

Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on a change in pull request #9223:
URL: https://github.com/apache/tvm/pull/9223#discussion_r731335329



##########
File path: src/target/llvm/llvm_common.cc
##########
@@ -106,10 +106,23 @@ void ParseLLVMTargetOptions(const Target& target, std::string* triple, std::stri
 #if TVM_LLVM_VERSION < 50
   opt.LessPreciseFPMADOption = true;
 #endif
-  opt.AllowFPOpFusion = llvm::FPOpFusion::Fast;
-  opt.UnsafeFPMath = false;
-  opt.NoInfsFPMath = false;
+  // We depend on generating IR with proper fast math flags to control fast math
+  // semantics. These just enable these optimizations if the proper IR flags
+  // are set.
+  opt.UnsafeFPMath = true;
+  opt.NoInfsFPMath = true;

Review comment:
       Hmm, yes these settings in Clang are passed in from LangOpts which describe the dialect of C or C++ that is accepted. Don't understand it fully and don't want to change the specification here so turned it off.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] AndrewZhaoLuo commented on pull request #9223: [Codegen][LLVM] Add ability to turn on fast math flags

Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on pull request #9223:
URL: https://github.com/apache/tvm/pull/9223#issuecomment-946188722


   PTAL @masahi 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] masahi commented on a change in pull request #9223: [Codegen][LLVM] Add ability to turn on fast math flags

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #9223:
URL: https://github.com/apache/tvm/pull/9223#discussion_r728534141



##########
File path: src/target/llvm/llvm_common.cc
##########
@@ -139,8 +152,22 @@ std::unique_ptr<llvm::TargetMachine> GetLLVMTargetMachine(const Target& target,
     ICHECK(allow_null) << err << " target_triple=" << target_triple;
     return nullptr;
   }
-  llvm::TargetMachine* tm =
-      llvm_target->createTargetMachine(target_triple, mcpu, mattr, opt, llvm::Reloc::PIC_);
+
+  Integer llvm_opt_level = target->GetAttr<Integer>("O").value_or(Integer(2));

Review comment:
       See https://github.com/apache/tvm/blob/3229cb329254764499dd672bb28fd9685ecd6a2e/src/target/llvm/codegen_llvm.cc#L346. I think they refer to the same opt level.
   
   I don't see why users would want to choose an opt level other than 3.  




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] AndrewZhaoLuo commented on pull request #9223: [Codegen][LLVM] Add ability to turn on fast math flags

Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on pull request #9223:
URL: https://github.com/apache/tvm/pull/9223#issuecomment-941702025


   This is ready for review now


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] masahi commented on a change in pull request #9223: [Codegen][LLVM] Add ability to turn on fast math flags

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #9223:
URL: https://github.com/apache/tvm/pull/9223#discussion_r728532649



##########
File path: src/target/llvm/llvm_common.cc
##########
@@ -106,10 +106,23 @@ void ParseLLVMTargetOptions(const Target& target, std::string* triple, std::stri
 #if TVM_LLVM_VERSION < 50
   opt.LessPreciseFPMADOption = true;
 #endif
-  opt.AllowFPOpFusion = llvm::FPOpFusion::Fast;
-  opt.UnsafeFPMath = false;
-  opt.NoInfsFPMath = false;
+  // We depend on generating IR with proper fast math flags to control fast math
+  // semantics. These just enable these optimizations if the proper IR flags
+  // are set.
+  opt.UnsafeFPMath = true;
+  opt.NoInfsFPMath = true;

Review comment:
       Better not to change the default values unless there is a good reason.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] masahi commented on a change in pull request #9223: [Codegen][LLVM] Add ability to turn on fast math flags

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #9223:
URL: https://github.com/apache/tvm/pull/9223#discussion_r731343221



##########
File path: src/target/llvm/codegen_llvm.cc
##########
@@ -343,7 +345,26 @@ void CodeGenLLVM::Optimize() {
 
   // place optimization pass
   llvm::PassManagerBuilder builder;
-  builder.OptLevel = 3;
+
+  // Use the same opt-level as specified in TargetMachine for running passes
+  llvm::CodeGenOpt::Level opt_level = target_machine_->getOptLevel();
+
+  switch (opt_level) {
+    case llvm::CodeGenOpt::Level::None:
+      builder.OptLevel = 0;
+      break;
+    case llvm::CodeGenOpt::Level::Less:
+      builder.OptLevel = 1;
+      break;
+
+    case llvm::CodeGenOpt::Level::Default:
+      builder.OptLevel = 2;
+      break;
+
+    default:
+      // CodeGenOpt::Level::Aggressive
+      builder.OptLevel = 3;

Review comment:
       A related comment: This code path should hit by default, otherwise this change would introduce regression.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] masahi commented on pull request #9223: [Codegen][LLVM] Add ability to turn on fast math flags

Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #9223:
URL: https://github.com/apache/tvm/pull/9223#issuecomment-946197480


   @AndrewZhaoLuo I think `-O=3` syntax is weird, how about `-opt-level=3`?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] AndrewZhaoLuo commented on a change in pull request #9223: [Codegen][LLVM] Add ability to turn on fast math flags

Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on a change in pull request #9223:
URL: https://github.com/apache/tvm/pull/9223#discussion_r731329410



##########
File path: src/target/llvm/llvm_common.cc
##########
@@ -139,8 +152,22 @@ std::unique_ptr<llvm::TargetMachine> GetLLVMTargetMachine(const Target& target,
     ICHECK(allow_null) << err << " target_triple=" << target_triple;
     return nullptr;
   }
-  llvm::TargetMachine* tm =
-      llvm_target->createTargetMachine(target_triple, mcpu, mattr, opt, llvm::Reloc::PIC_);
+
+  Integer llvm_opt_level = target->GetAttr<Integer>("O").value_or(Integer(2));

Review comment:
       I made it based on the -O flag added. Default is still 2 however (which is the default for clang I believe)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] AndrewZhaoLuo commented on pull request #9223: [Codegen][LLVM] Add ability to turn on fast math flags

Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on pull request #9223:
URL: https://github.com/apache/tvm/pull/9223#issuecomment-946199593


   Done, renamed -O --> -opt-level


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [tvm] masahi commented on a change in pull request #9223: [Codegen][LLVM] Add ability to turn on fast math flags

Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #9223:
URL: https://github.com/apache/tvm/pull/9223#discussion_r731347240



##########
File path: src/target/llvm/codegen_llvm.cc
##########
@@ -343,7 +345,26 @@ void CodeGenLLVM::Optimize() {
 
   // place optimization pass
   llvm::PassManagerBuilder builder;
-  builder.OptLevel = 3;
+
+  // Use the same opt-level as specified in TargetMachine for running passes
+  llvm::CodeGenOpt::Level opt_level = target_machine_->getOptLevel();
+
+  switch (opt_level) {
+    case llvm::CodeGenOpt::Level::None:
+      builder.OptLevel = 0;
+      break;
+    case llvm::CodeGenOpt::Level::Less:
+      builder.OptLevel = 1;
+      break;
+
+    case llvm::CodeGenOpt::Level::Default:
+      builder.OptLevel = 2;
+      break;
+
+    default:
+      // CodeGenOpt::Level::Aggressive
+      builder.OptLevel = 3;

Review comment:
       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




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org