kparzysz-quic commented on a change in pull request #10586:
URL: https://github.com/apache/tvm/pull/10586#discussion_r825488960



##########
File path: src/target/llvm/codegen_hexagon.cc
##########
@@ -76,6 +76,8 @@ class CodeGenHexagon final : public CodeGenLLVM {
   llvm::FunctionType* ftype_tvm_api_set_last_error_{nullptr};
 
  private:
+  TypedPointer CreateBufferPtr(llvm::Value* buffer_ptr, DataType 
buffer_element_dtype,
+                               std::vector<llvm::Value*> indices, DataType 
value_dtype) final;

Review comment:
       You can use `llvm::ArrayRef<llvm::Value*>` instead of `std::vector`, 
it's a bit more lightweight structure for non-modifiable arrays.

##########
File path: src/target/llvm/codegen_hexagon.cc
##########
@@ -570,6 +572,31 @@ llvm::Value* CodeGenHexagon::CreateIntrinsic(const 
CallNode* op) {
   return CodeGenLLVM::CreateIntrinsic(op);
 }
 
+CodeGenLLVM::TypedPointer CodeGenHexagon::CreateBufferPtr(llvm::Value* 
buffer_ptr,
+                                                          DataType 
buffer_element_dtype,
+                                                          
std::vector<llvm::Value*> indices,

Review comment:
       Same here.

##########
File path: src/target/llvm/codegen_llvm.cc
##########
@@ -1274,39 +1278,56 @@ bool CodeGenLLVM::HasAlignmentPadding(DataType dtype) {
 }
 
 void CodeGenLLVM::BufferAccessHelper(
-    Buffer buffer, PrimExpr index, DataType value_dtype,
+    Buffer buffer, Array<PrimExpr> indices, DataType value_dtype,
     std::function<llvm::Instruction*(TypedPointer buffer_ptr, int 
subelement_i, int alignment,
                                      bool is_volatile)>
         make_instruction) {
   DataType buffer_element_dtype = buffer->dtype;
 
-  ICHECK_EQ(value_dtype.lanes(), index.dtype().lanes() * 
buffer_element_dtype.lanes());
+  // Only the last index is allowed to be multi-lane.  All earlier
+  // indices must be scalar.  This only matters for subclasses of
+  // CodeGenLLVM, because the default implementation of GetBufferPtr
+  // requires 1-d indices.
+  std::vector<llvm::Value*> earlier_index_values;
+  for (size_t i = 0; i < indices.size() - 1; i++) {
+    ICHECK_EQ(indices[i].dtype().lanes(), 1)
+        << "Buffer " << buffer->name << " is accessed with a multi-lane index 
at position " << i
+        << ".  Multi-lane indices are only supported as the last index.";
+    earlier_index_values.push_back(MakeValue(indices[i]));
+  }
+
+  ICHECK_GE(indices.size(), 1)

Review comment:
       Please move this check to the front.  Otherwise the condition `i < 
indices.size() - 1` at line 1292 may cause trouble before you detect it.




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


Reply via email to