Skip to content

[WIP] Buffer strides are required by tensorize - #1725

Closed
yzhliu wants to merge 1 commit into
apache:masterfrom
yzhliu:tensor_intrin_strides
Closed

yzhliu wants to merge 1 commit into
apache:masterfrom
yzhliu:tensor_intrin_strides

Conversation

@yzhliu

@yzhliu yzhliu commented Sep 17, 2018

Copy link
Copy Markdown
Member

If strides are not specified, the vload, vstore stuff will calculate buffer offset as https://github.com/dmlc/tvm/blob/master/src/lang/buffer.cc#L227-L239, but in tensorize, the tensor/buffer is passed from outside, with start address already includes the axes in the intrinsic, thus results the incorrect offset.

For example,

def intrinc(in, outs):
  data = tvm.placeholder((d2,d3), dtype='uint8', name='data')
  data.vload([i2, i3])

Input = tvm.placeholder((d1, d2, d3))
s[Input].tensorize(Input.axis[1], intrinc) 

then the start offset passed to intrinc is (i1 * d2 + 0) * d3 + 0, without strides, vload will (incorrectly) return offset ((i1 * d2 + 0) * d3 + i2) * d3 + i3

It is confusing to users to understand, in most cases, tensorize results will be incorrect without binding a buffer with strides.

Will come with test case and doc improvements.

Comment thread python/tvm/schedule.py
begin = (begin,) if isinstance(begin, (int, _expr.Expr)) else begin
return _api_internal._BufferVStore(self, begin, value)

def make_stride_view(self):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

document this function

@tqchen

tqchen commented Sep 17, 2018

Copy link
Copy Markdown
Member

The strides requirement really specifies what is the requirement of the buffer. If there is no strides, then the data is expected to be continuous, and tensorize should raise an error if the data being matched is not continuous

@tqchen

tqchen commented Sep 17, 2018

Copy link
Copy Markdown
Member

@ZihengJiang can you also take a look at this?

@ZihengJiang ZihengJiang self-assigned this Sep 17, 2018
@ZihengJiang

Copy link
Copy Markdown
Contributor

ye, I always feel confused about this too

@tqchen

tqchen commented Sep 20, 2018

Copy link
Copy Markdown
Member

Hmm, maybe we should add a detailed document page about the differences

@tqchen

tqchen commented Sep 20, 2018

Copy link
Copy Markdown
Member

some followup discussion here as well https://discuss.tvm.ai/t/how-to-use-tensorize/424/6

@yzhliu

yzhliu commented Sep 23, 2018

Copy link
Copy Markdown
Member Author

After offline discussing with Tianqi, this is problem with elem_offset calculation itself. Ref: #1762

@yzhliu yzhliu closed this Sep 23, 2018
@yzhliu
yzhliu deleted the tensor_intrin_strides branch September 25, 2018 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants