-
Notifications
You must be signed in to change notification settings - Fork 157
layout: Add support for permutation layout in unpackOp #3157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| // RUN: heir-opt --lower-unpack --canonicalize %s | FileCheck %s | ||
|
|
||
| // Tests `tensor_ext.unpack` with a dense-permutation layout attribute | ||
|
|
||
| // ----------------------------------------------------------------------------- | ||
| // Case 1: single dense permutation with a rank-1 data-semantic target. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the design of this explicit layout attribute is insufficient for what you're trying to use it for, and this PR is showing the strain. The original permutation attribute seems incorrect in retrospect: In fact, it's not a mapping (ct, slot) -> (ct, slot), because one side is data semantic (and restricted to rank 2) and the other is ciphertext semantic. And then here, one must further hack around it by having the permutation attribute use rank 2 indices for the data semantic side, even though it has semantic rank 1. Probably what it should be is With a verifier to assert that when this is attached to an op, the ranks agree (the data semantic tensor involved has rank Then the layout attribute would naturally support any rank of data-semantic tensor.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes that seems like a more generic implementation. But the subtle issue with
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't see why not...
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
because for a result attribute, we are going from ciphertext to a n-dim array, hence we would need
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The layout attribute is defined to always map from the data-semantic domain to the ciphertext-semantic domain. See for example https://github.com/google/heir/blob/main/tests/Transforms/lower_unpack/lower_unpack.mlir Then |
||
| // | ||
| // The permutation `[[0, 3, 0, 0], [0, 5, 0, 1], [0, 1, 0, 2]]` gathers slots | ||
| // 3, 5, 1 from a single ciphertext view (rank-2, one row) into positions | ||
| // 0, 1, 2 of a rank-1 result. | ||
|
|
||
| #dense_perm_single = dense<[[0, 3, 0, 0], [0, 5, 0, 1], [0, 1, 0, 2]]> | ||
| : tensor<3x4xi64> | ||
| #orig_single = #tensor_ext.original_type< | ||
| originalType = tensor<3xi32>, layout = #dense_perm_single> | ||
|
|
||
| // CHECK: @unpack_dense_single | ||
| // CHECK-DAG: %[[C0:.*]] = arith.constant 0 : index | ||
| // CHECK-DAG: %[[C1:.*]] = arith.constant 1 : index | ||
| // CHECK-DAG: %[[C2:.*]] = arith.constant 2 : index | ||
| // CHECK-DAG: %[[C3:.*]] = arith.constant 3 : index | ||
| // CHECK-DAG: %[[C5:.*]] = arith.constant 5 : index | ||
| // CHECK-DAG: %[[ZERO:.*]] = arith.constant dense<0> : tensor<3xi32> | ||
| // CHECK-DAG: %[[E0:.*]] = tensor.extract %arg0[%[[C0]], %[[C3]]] : tensor<1x8xi32> | ||
| // CHECK-DAG: tensor.insert %[[E0]] into %{{.*}}[%[[C0]]] : tensor<3xi32> | ||
| // CHECK-DAG: %[[E1:.*]] = tensor.extract %arg0[%[[C0]], %[[C5]]] : tensor<1x8xi32> | ||
| // CHECK-DAG: tensor.insert %[[E1]] into %{{.*}}[%[[C1]]] : tensor<3xi32> | ||
| // CHECK-DAG: %[[E2:.*]] = tensor.extract %arg0[%[[C0]], %[[C1]]] : tensor<1x8xi32> | ||
| // CHECK-DAG: tensor.insert %[[E2]] into %{{.*}}[%[[C2]]] : tensor<3xi32> | ||
| // CHECK: return %{{.*}} : tensor<3xi32> | ||
| func.func @unpack_dense_single( | ||
| %arg0: tensor<1x8xi32> {tensor_ext.original_type = #orig_single} | ||
| ) -> tensor<3xi32> { | ||
| %0 = tensor_ext.unpack %arg0 {layout = #dense_perm_single} | ||
| : (tensor<1x8xi32>) -> tensor<3xi32> | ||
| return %0 : tensor<3xi32> | ||
| } | ||
|
|
||
| // ----------------------------------------------------------------------------- | ||
| // Case 2: dense permutation with cross-ct source. The permutation | ||
| // `[[1, 2, 0, 0], [0, 4, 0, 1]]` says logical output 0 lives at (ct=1, slot=2) | ||
| // and logical output 1 lives at (ct=0, slot=4). | ||
|
|
||
| #dense_perm_cross = dense<[[1, 2, 0, 0], [0, 4, 0, 1]]> : tensor<2x4xi64> | ||
| #orig_cross = #tensor_ext.original_type< | ||
| originalType = tensor<2xi32>, layout = #dense_perm_cross> | ||
|
|
||
| // CHECK: @unpack_dense_cross_ct | ||
| // CHECK-DAG: %[[C0:.*]] = arith.constant 0 : index | ||
| // CHECK-DAG: %[[C1:.*]] = arith.constant 1 : index | ||
| // CHECK-DAG: %[[C2:.*]] = arith.constant 2 : index | ||
| // CHECK-DAG: %[[C4:.*]] = arith.constant 4 : index | ||
| // CHECK-DAG: %[[E0:.*]] = tensor.extract %arg0[%[[C1]], %[[C2]]] : tensor<2x8xi32> | ||
| // CHECK-DAG: tensor.insert %[[E0]] into %{{.*}}[%[[C0]]] : tensor<2xi32> | ||
| // CHECK-DAG: %[[E1:.*]] = tensor.extract %arg0[%[[C0]], %[[C4]]] : tensor<2x8xi32> | ||
| // CHECK-DAG: tensor.insert %[[E1]] into %{{.*}}[%[[C1]]] : tensor<2xi32> | ||
| // CHECK: return %{{.*}} : tensor<2xi32> | ||
| func.func @unpack_dense_cross_ct( | ||
| %arg0: tensor<2x8xi32> {tensor_ext.original_type = #orig_cross} | ||
| ) -> tensor<2xi32> { | ||
| %0 = tensor_ext.unpack %arg0 {layout = #dense_perm_cross} | ||
| : (tensor<2x8xi32>) -> tensor<2xi32> | ||
| return %0 : tensor<2xi32> | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The naming is a little strange here because only one of the source and destination are semantically "ct, slot," depending on the caller's context. Moreover, the non-ct-slot side is restricted (somewhat arbitrarily) to be dimension 2.
Maybe don't name the variables "ct, slot", and then document the function with a code comment for the dim-2 restriction and for the semantic ambiguity of which one is the data and which is ciphertext.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized this, I can rework this when I address the overall issue with the design of layout attribute.