gemini-code-assist[bot] commented on code in PR #19877:
URL: https://github.com/apache/tvm/pull/19877#discussion_r3462254280


##########
src/target/llvm/llvm_instance.cc:
##########
@@ -199,13 +277,26 @@ LLVMTargetInfo::LLVMTargetInfo(LLVMInstance& instance,
   if (triple_.empty() || triple_ == "default") {
     triple_ = llvm::sys::getDefaultTargetTriple();
   }
+  bool has_explicit_mcpu = target.Get("mcpu").has_value();
   cpu_ = 
Downcast<ffi::String>(target.Get("mcpu").value_or(ffi::String(defaults::cpu)));
 
+  bool has_explicit_mattr = false;
   if (const auto& v = 
Downcast<ffi::Optional<ffi::Array<ffi::String>>>(target.Get("mattr"))) {
+    has_explicit_mattr = true;
     for (const ffi::String& s : v.value()) {
       attrs_.push_back(s);
     }
   }
+
+  auto arch = llvm::Triple(triple_).getArch();
+  if (!has_explicit_mattr && !has_explicit_mcpu &&
+      (arch == llvm::Triple::riscv32 || arch == llvm::Triple::riscv64)) {
+    std::vector<std::string> detected_attrs = DetectLocalRISCVAttrs();
+    for (const std::string& attr : detected_attrs) {
+      attrs_.push_back(attr);
+    }
+  }

Review Comment:
   ![high](https://www.gstatic.com/codereviewagent/high-priority.svg)
   
   Automatically detecting and applying local CPU features when `mcpu` and 
`mattr` are not specified is a great feature, but it should only be done when 
targeting the host machine. If a user is cross-compiling on a RISC-V host for a 
different RISC-V target (for example, compiling for a 32-bit bare-metal target 
`riscv32-unknown-elf` from a 64-bit RISC-V Linux host), detecting and applying 
the host's 64-bit features (like double-precision float `+d` or vector 
extensions `+v`) will result in generating unsupported instructions for the 
target device.
   
   To prevent this, we should verify that the target triple's architecture and 
OS match the host's architecture and OS before applying local CPU features.
   
   ```c
     llvm::Triple target_triple(triple_);
     auto arch = target_triple.getArch();
     llvm::Triple host_triple(llvm::sys::getDefaultTargetTriple());
     if (!has_explicit_mattr && !has_explicit_mcpu &&
         arch == host_triple.getArch() &&
         target_triple.getOS() == host_triple.getOS() &&
         (arch == llvm::Triple::riscv32 || arch == llvm::Triple::riscv64)) {
       std::vector<std::string> detected_attrs = DetectLocalRISCVAttrs();
       for (const std::string& attr : detected_attrs) {
         attrs_.push_back(attr);
       }
     }
   ```



-- 
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]

Reply via email to