cbalint13 commented on code in PR #18586:
URL: https://github.com/apache/tvm/pull/18586#discussion_r2617613861


##########
src/target/llvm/codegen_aarch64.cc:
##########
@@ -59,9 +59,11 @@ void CodeGenAArch64::SetTargetAttributes(llvm::Function* 
func) {
   // Add vscale_range() function attribute when appropriate.
   if (llvm_target_->TargetHasCPUFeature("sve") || 
llvm_target_->TargetHasCPUFeature("sme")) {
     auto kVScaleValues = arith::GetVScaleValues(Target::Current());
-    unsigned int max_val = *std::max_element(kVScaleValues.begin(), 
kVScaleValues.end());
-    func->addFnAttr(
-        llvm::Attribute::getWithVScaleRangeArgs(*llvm_target_->GetContext(), 
1, max_val));
+    if (!kVScaleValues.empty()) {

Review Comment:
   It would be fine to protect this way, but we will miss the origin of issue.
   
   My concern is that if we have sve/sme present why the list is empty !?, 
   For a proper fix, I would be courious what is the string value of 
```target``` and what value have ```vector_width``` Here is the generator part 
that populates: 
https://github.com/apache/tvm/blame/6248b5db43505fbcfb13cc289d11877d5d2649e8/src/arith/scalable_expression.cc#L114
 .
   
   I am afraid that TVM fails to pickup proper ```vector_width``` during 
initial parsing of arch properties based on target provided.
   
   If could help me with these two info i can come up with a proper fix,  I 
could also go the long way to test it on arm machine but than I need a clear 
reproducer case.



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