-
Notifications
You must be signed in to change notification settings - Fork 238
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
[hail/ptypes] copy based on ptype and requiredeness #7639
Changes from all commits
3e02787
da6eb1a
aeb525f
3532dcd
da26541
d6206e2
bca7611
adaebe8
42d62f3
e0fdd06
af058f7
69837e9
5201abc
b23fde9
0508371
0b5eb1f
125ab0d
6415b75
1ac9441
3b11997
5bc1f64
d4adb22
34abefe
dcfca5e
8e478a8
e72ddcb
0248b8d
0292be5
a5d650c
faa0fbb
e99d6b5
0fa1156
9af2ae3
26e9169
e8ed486
8ff741b
a342552
4810ee3
bab7137
cbbda64
f9617d0
6331b50
5b731e8
4160ef7
7d0c8e3
a85a456
187168c
5741984
5590ed4
dec1cf6
0935f6c
f9b2189
6332522
83c3898
7552aeb
eea41b3
91f2e05
2ebae40
da7f6fb
a6d62ce
00af1bb
0978fa7
56ad469
b0c1058
4b802ac
f51ab95
492965d
e28dd16
157f498
4a604a6
ca3c5b1
3add474
ba36d89
ef4e60a
8884e41
c3503e6
5fbfe42
c71ac66
1e2debc
d4a0f30
921c7e8
9a83509
1346f3e
87ce25d
cddc0a6
395d831
a4ff53e
19df4c9
e813488
596b9a1
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 |
---|---|---|
@@ -1,7 +1,7 @@ | ||
package is.hail.expr.types.physical | ||
|
||
import is.hail.annotations.{Region, UnsafeOrdering} | ||
import is.hail.asm4s.{Code, MethodBuilder} | ||
import is.hail.asm4s.{???, Code, MethodBuilder} | ||
import is.hail.expr.ir.EmitMethodBuilder | ||
|
||
trait PArrayBackedContainer extends PContainer { | ||
|
@@ -110,6 +110,9 @@ trait PArrayBackedContainer extends PContainer { | |
def loadElement(aoff: Long, length: Int, i: Int): Long = | ||
arrayRep.loadElement(aoff, length, i) | ||
|
||
def loadElementAddress(aoff: Code[Long], length: Code[Int], i: Code[Int]): Code[Long] = | ||
arrayRep.loadElementAddress(aoff, length, i) | ||
|
||
def loadElement(region: Region, aoff: Long, length: Int, i: Int): Long = | ||
arrayRep.loadElement(region, aoff, length, i) | ||
|
||
|
@@ -164,7 +167,29 @@ trait PArrayBackedContainer extends PContainer { | |
def copyFrom(mb: MethodBuilder, region: Code[Region], srcOff: Code[Long]): Code[Long] = | ||
arrayRep.copyFrom(mb, region, srcOff) | ||
|
||
override def unsafeOrdering: UnsafeOrdering = unsafeOrdering(this) | ||
override def unsafeOrdering: UnsafeOrdering = | ||
unsafeOrdering(this) | ||
|
||
override def unsafeOrdering(rightType: PType): UnsafeOrdering = | ||
arrayRep.unsafeOrdering(rightType) | ||
|
||
override def copyFromType(mb: MethodBuilder, region: Code[Region], srcPType: PType, srcAddress: Code[Long], forceDeep: Boolean): Code[Long] = { | ||
assert(srcPType.isInstanceOf[PArrayBackedContainer]) | ||
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. We could be more general here, allow copying with a PArray as well. |
||
this.arrayRep.copyFromType(mb, region, srcPType.asInstanceOf[PArrayBackedContainer].arrayRep, srcAddress, forceDeep) | ||
} | ||
|
||
override def copyFromType(region: Region, srcPType: PType, srcAddress: Long, forceDeep: Boolean): Long = ??? | ||
|
||
override def storeShallowAtOffset(dstAddress: Code[Long], valueAddress: Code[Long]): Code[Unit] = | ||
this.arrayRep.storeShallowAtOffset(dstAddress, valueAddress) | ||
|
||
override def storeShallowAtOffset(dstAddress: Long, valueAddress: Long) { | ||
this.arrayRep.storeShallowAtOffset(dstAddress, valueAddress) | ||
} | ||
|
||
def nextElementAddress(currentOffset: Long) = | ||
arrayRep.nextElementAddress(currentOffset) | ||
|
||
override def unsafeOrdering(rightType: PType): UnsafeOrdering = arrayRep.unsafeOrdering(rightType) | ||
def nextElementAddress(currentOffset: Code[Long]) = | ||
arrayRep.nextElementAddress(currentOffset) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -74,6 +74,7 @@ abstract class PBaseStruct extends PType { | |
} | ||
|
||
def identBase: String | ||
|
||
def _asIdent: String = { | ||
val sb = new StringBuilder | ||
sb.append(identBase) | ||
|
@@ -147,7 +148,30 @@ abstract class PBaseStruct extends PType { | |
region.allocate(alignment, byteSize) | ||
} | ||
|
||
def allocate(region: Code[Region]): Code[Long] = region.allocate(alignment, byteSize) | ||
def allocate(region: Code[Region]): Code[Long] = | ||
region.allocate(alignment, byteSize) | ||
|
||
def copyFrom(region: Region, srcAddress: Long): Long = { | ||
val destAddress = this.allocate(region) | ||
this.storeShallowAtOffset(destAddress, srcAddress) | ||
destAddress | ||
} | ||
|
||
def copyFrom(mb: MethodBuilder, region: Code[Region], srcAddress: Code[Long]): Code[Long] = { | ||
val destAddress = mb.newField[Long] | ||
Code( | ||
destAddress := this.allocate(region), | ||
this.storeShallowAtOffset(destAddress, srcAddress), | ||
destAddress | ||
) | ||
} | ||
|
||
override def storeShallowAtOffset(destAddress: Code[Long], srcAddress: Code[Long]): Code[Unit] = | ||
Region.copyFrom(srcAddress, destAddress, this.byteSize) | ||
|
||
override def storeShallowAtOffset(destAddress: Long, srcAddress: Long) { | ||
Region.copyFrom(srcAddress, destAddress, this.byteSize) | ||
} | ||
|
||
def initialize(structAddress: Long, setMissing: Boolean = false): Unit = { | ||
if (allFieldsRequired) { | ||
|
@@ -217,11 +241,11 @@ abstract class PBaseStruct extends PType { | |
def setFieldPresent(region: Code[Region], offset: Code[Long], fieldIdx: Int): Code[Unit] = | ||
setFieldPresent(offset, fieldIdx) | ||
|
||
def fieldOffset(offset: Long, fieldIdx: Int): Long = | ||
offset + byteOffsets(fieldIdx) | ||
def fieldOffset(structAddress: Long, fieldIdx: Int): Long = | ||
structAddress + byteOffsets(fieldIdx) | ||
|
||
def fieldOffset(offset: Code[Long], fieldIdx: Int): Code[Long] = | ||
offset + byteOffsets(fieldIdx) | ||
def fieldOffset(structAddress: Code[Long], fieldIdx: Int): Code[Long] = | ||
structAddress + byteOffsets(fieldIdx) | ||
|
||
def loadField(rv: RegionValue, fieldIdx: Int): Long = loadField(rv.region, rv.offset, fieldIdx) | ||
|
||
|
@@ -248,5 +272,107 @@ abstract class PBaseStruct extends PType { | |
} | ||
} | ||
|
||
def deepCopyFromAddress(mb: MethodBuilder, region: Code[Region], srcStructAddress: Code[Long]): Code[Long] = { | ||
val dstAddress = mb.newField[Long] | ||
Code( | ||
dstAddress := this.copyFrom(mb, region, srcStructAddress), | ||
this.deepPointerCopy(mb, region, dstAddress), | ||
dstAddress | ||
) | ||
} | ||
|
||
def deepPointerCopy(mb: MethodBuilder, region: Code[Region], dstStructAddress: Code[Long]): Code[Unit] = { | ||
var c: Code[Unit] = Code._empty | ||
|
||
var i = 0 | ||
while(i < this.size) { | ||
val dstFieldType = this.fields(i).typ.fundamentalType | ||
if(dstFieldType.containsPointers) { | ||
val dstFieldAddress = mb.newField[Long] | ||
c = Code( | ||
c, | ||
this.isFieldDefined(dstStructAddress, i).orEmpty( | ||
Code( | ||
dstFieldAddress := this.fieldOffset(dstStructAddress, i), | ||
dstFieldType match { | ||
case t@(_: PBinary | _: PArray) => | ||
t.storeShallowAtOffset(dstFieldAddress, t.copyFromType(mb, region, dstFieldType, Region.loadAddress(dstFieldAddress))) | ||
case t: PBaseStruct => | ||
t.deepPointerCopy(mb, region, dstFieldAddress) | ||
case t: PType => | ||
fatal(s"Field type isn't supported ${t}") | ||
} | ||
) | ||
) | ||
) | ||
} | ||
i += 1 | ||
} | ||
|
||
c | ||
} | ||
|
||
override def copyFromType(mb: MethodBuilder, region: Code[Region], srcPType: PType, srcStructAddress: Code[Long], | ||
forceDeep: Boolean): Code[Long] = { | ||
assert(srcPType.isInstanceOf[PBaseStruct]) | ||
|
||
val sourceType = srcPType.asInstanceOf[PBaseStruct] | ||
|
||
assert(sourceType.size == this.size) | ||
|
||
val sourceFundamentalTypes = sourceType.fields.map(_.typ.fundamentalType) | ||
if(this.fields.map(_.typ.fundamentalType) == sourceFundamentalTypes) { | ||
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. this check seems insufficient to me. What if there are two different PStruct types with teh same fields? You can't copy the address in this case. 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. Would 2 different case classes that happened to have the same attribute values be equal? Meaning PStruct(fields: Seq("1" -> 2)) == PNonCanonicalStruct(fields: Seq("1"->2))? That seems very odd if so, I imagine (I think the equality is hash based), the value that is hashed included the class name (and probably much more metadata) |
||
if(!forceDeep) { | ||
return srcStructAddress | ||
} | ||
|
||
return this.deepCopyFromAddress(mb, region, srcStructAddress) | ||
} | ||
|
||
val dstStructAddress = mb.newField[Long] | ||
var loop: Code[_] = Code() | ||
var i = 0 | ||
while(i < this.size) { | ||
val dstField = this.fields(i) | ||
val srcField = sourceType.fields(i) | ||
|
||
assert((dstField.typ.required <= srcField.typ.required) && (dstField.typ isOfType srcField.typ) && (dstField.name == srcField.name) && (dstField.index == srcField.index)) | ||
|
||
val srcFieldType = srcField.typ.fundamentalType | ||
val dstFieldType = dstField.typ.fundamentalType | ||
|
||
val body = srcFieldType.storeShallowAtOffset( | ||
this.fieldOffset(dstStructAddress, dstField.index), | ||
dstFieldType.copyFromType( | ||
mb, | ||
region, | ||
srcFieldType, | ||
sourceType.loadField(srcStructAddress, srcField.index), | ||
forceDeep | ||
) | ||
) | ||
|
||
if(!srcFieldType.required) { | ||
loop = Code(loop, sourceType.isFieldMissing(srcStructAddress, srcField.index).mux( | ||
this.setFieldMissing(dstStructAddress, dstField.index), | ||
body | ||
)) | ||
} else { | ||
loop = Code(loop, body) | ||
} | ||
|
||
i+=1 | ||
} | ||
|
||
Code( | ||
dstStructAddress := this.allocate(region), | ||
this.stagedInitialize(dstStructAddress), | ||
loop, | ||
dstStructAddress | ||
) | ||
} | ||
|
||
override def copyFromType(region: Region, srcPType: PType, srcAddress: Long, forceDeep: Boolean): Long = ??? | ||
|
||
override def containsPointers: Boolean = types.exists(_.containsPointers) | ||
} |
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.
needed to be called something other than loadElement, because scala cannot distinguish between Code[Region] and Code[Long] types in the argument list, and besides this (arity, other argument types), this method is identical to loadElement(region: Code[Region] ...), leading to type check errors.
Later on, in the PR corresponding to #7826, the region-bearing methods will go away.
references #7829