gemini-code-assist[bot] commented on code in PR #20025:
URL: https://github.com/apache/tvm/pull/20025#discussion_r3603296754
##########
src/target/llvm/codegen_llvm.cc:
##########
@@ -376,9 +377,38 @@ std::unique_ptr<llvm::Module> CodeGenLLVM::Finish() {
<< "Failed to link modules";
}
link_modules_.clear();
+
+ // Verify the freshly generated (pre-optimization) module. This is TVM's own
+ // codegen output; if it fails to verify that is a genuine TVM bug, so keep
+ // throwing here.
this->Verify();
+
+ // LLVM's optimizer has been observed to miscompile perfectly valid IR into a
+ // structurally-invalid module for certain shapes -- e.g. avg_pool2d with
+ // C == 4 and W == 3 sends LLVM's vectorizer down a buggy path that emits a
+ // shufflevector whose definition does not dominate its use (TVM issue
#20015;
+ // reproducible with stock `opt -passes='default<O2>'`, an upstream LLVM
bug).
+ // To avoid hard-crashing the whole build on such backend-optimizer bugs,
keep
+ // a verified copy of the pre-optimization module and fall back to it if
+ // optimization produces an invalid module. The result is correct (just
+ // unoptimized for the affected module) rather than a crash, and we never
emit
+ // the broken module.
+ std::unique_ptr<llvm::Module> pre_opt_module = llvm::CloneModule(*module_);
+
this->Optimize();
- this->Verify();
+
+ std::string verify_errors_storage;
+ llvm::raw_string_ostream verify_errors(verify_errors_storage);
+ if (llvm::verifyModule(*module_, &verify_errors)) {
+ LOG(WARNING) << "LLVM " << (TVM_LLVM_VERSION / 10) << "." <<
(TVM_LLVM_VERSION % 10)
+ << " optimization produced an invalid module; this is a known
LLVM "
+ "optimizer bug on valid IR (see TVM issue #20015). Falling
back to the "
+ "unoptimized module so the build can proceed with correct
results.\n"
+ << "Verifier errors:\n"
+ << verify_errors.str();
+ module_ = std::move(pre_opt_module);
+ }
Review Comment:
Unconditionally cloning the LLVM module on every compilation can introduce
unnecessary overhead, especially when optimization is disabled (at `O0`). Since
the optimizer bug only occurs at higher optimization levels, we can avoid this
cloning overhead when the optimization level is `None` (0). If verification
fails at `O0` (where we didn't clone), we can throw a proper error instead of
falling back.
```c
std::unique_ptr<llvm::Module> pre_opt_module;
if (static_cast<int>(llvm_target_->GetOptLevel()) > 0) {
pre_opt_module = llvm::CloneModule(*module_);
}
this->Optimize();
std::string verify_errors_storage;
llvm::raw_string_ostream verify_errors(verify_errors_storage);
if (llvm::verifyModule(*module_, &verify_errors)) {
if (pre_opt_module) {
LOG(WARNING) << "LLVM " << (TVM_LLVM_VERSION / 10) << "." <<
(TVM_LLVM_VERSION % 10)
<< " optimization produced an invalid module; this is a
known LLVM "
"optimizer bug on valid IR (see TVM issue #20015).
Falling back to the "
"unoptimized module so the build can proceed with
correct results.\n"
<< "Verifier errors:\n"
<< verify_errors.str();
module_ = std::move(pre_opt_module);
} else {
TVM_FFI_THROW(InternalError) << "LLVM module verification failed after
optimization: \n"
<< verify_errors.str();
}
}
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
