From: Kris V. <kri...@el...> - 2005-01-21 00:38:23
|
There has been a commit that screwed the 64-bit VM. Can someone please roll back the following commits of which I'm sure they are erroneous : Commit on 18th jan of mmtk/utility/Memory.java : uncommit entirely, except for the import statements. You can`t substitute loadInt with loadWord without consequences for 64-bit. They are NOT the same. Commit on 18th jan of JMTk/vmInterface/ScanStatics.java: undo the change of LOG_BYTES_IN_INT into LOG_BYTES_IN_ADDRESS. It really needs to be LOG_BYTES_IN_INT. regards, Kris Venstermans ELIS Department, University of Ghent, Belgium. |
From: Kris V. <Kris.Venstermans@UGent.be> - 2005-01-25 18:02:04
|
>I would like to ask those who did the 64 bit port to try to precisely >identify what the alignment assumptions and dependencies, so that the >code can be properly documented and uses of BYTES_IN_INT can be >replaced >with BYTES_IN_PARTICLE where appropriate. I'll give it a start: in MMTk/src/org/mmtk/utility/alloc/Allocator.java are some asserts. Is it OK that the constants in rvm/src/vm/memoryManagers/JMTk/vmInterface/VMConstants.java and MMTk/src/org/mmtk/vm/VMConstants.java are not the same? I find it at least confusing. Kris. |
From: Steven A. <au...@wa...> - 2005-01-25 19:08:29
|
Kris Venstermans wrote: > >I would like to ask those who did the 64 bit port to try to precisely > >identify what the alignment assumptions and dependencies, so that the > >code can be properly documented and uses of BYTES_IN_INT can be >replaced > >with BYTES_IN_PARTICLE where appropriate. > > I'll give it a start: > in MMTk/src/org/mmtk/utility/alloc/Allocator.java are some asserts. > > > Is it OK that the constants in > rvm/src/vm/memoryManagers/JMTk/vmInterface/VMConstants.java and > MMTk/src/org/mmtk/vm/VMConstants.java > are not the same? I find it at least confusing. I don't know if this is the answer you want (you may be looking for something deeper than this), but: The implementations in MMTk/src/org/mmtk/vm are sample implementations for some sort of generic VM. The idea is that each VM will implement its own version of these. So, the versions in rvm/src/vm/memoryManagers/JMTk/vmInterface override the ones in the master MMTk sources. I certainly agree that it would help to have a README file in the directory MMTk/src/org/mmtk/vm and a nice fat comment at the start of each of the sample implementations there. MMTk guys, would one of you be able to do this? --Steve Augart |
From: Steve B. <Ste...@an...> - 2005-01-25 23:33:14
|
> I'll give it a start: > in MMTk/src/org/mmtk/utility/alloc/Allocator.java are some asserts. Yes, but these are merely the artifact of some assumptions. The basic assumption is that allocation is BYTES_IN_PARTICLE aligned. My original question was what was the relationship between BYTES_IN_PARTICLE, BYTES_IN_INT, BYTES_IN_WORD and the choice of 64 & 32 bit. I think this remains unanswered and our initial question about Memory.java and its use of BYTES_IN_INT similarly remains unanswered. > Is it OK that the constants in > rvm/src/vm/memoryManagers/JMTk/vmInterface/VMConstants.java and > MMTk/src/org/mmtk/vm/VMConstants.java > are not the same? I find it at least confusing. Of course. The files in org/mmtk/vm are simply VM-neutral stubs. Note that the interface to VMConstants is not actually constants, but methods that look like constants. This allows the MMTk core to be compiled against stub versions of org/mmtk/vm (the VM-specific aspect of MMTk) without javac's constant propogation compiling in the dummy constants. Thus we can build a jar file that is VM-neutral and then include that with any VM. I can add some comments to make this clearer. --Steve |
From: Steve B. <Ste...@an...> - 2005-01-25 23:34:44
|
I should add, that as far as I can tell, the relationship between BYTES_IN_PARTICLE and BYTES_IN_INT is Jikes RVM - specific, and therefore should not be exposed within the core mmtk code... --Steve |
From: David P G. <gr...@us...> - 2005-01-25 23:46:44
|
I think the high level question is do we really want BYTES_IN_PARTICLE != BYTES_IN_WORD? Under some object models, there is an advantage in 64 bit land to be able to "mis-align" the lowest word of the object so that a subsequent field (eg at offset 4 in the object) can be 64bit aligned. I think it is likely that with the switch to the forward scalar object model and the current placement of the array length to the "right" of the TIB, that there is no longer an advantage in doing this. If it makes the code simpler/cleaner than my suggestion would be that we drop the distinction between BYTES_IN_PARTICLE and BYTES_IN_WORD. There are still cases where the VM is going to want to allocate storage that (at a known offset from the start of the allocated region) is allocated to a larger unit than BYTES_IN_WORD (for example double[] in 32 bit mode and scalar objects that contain long/double fields in 32 bit mode). So, we need this ability, but subword alignment in 64 bit mode may not be that useful (at least under the current object model). --dave |
From: Eliot M. <mo...@cs...> - 2005-01-26 00:16:58
|
>>>>> "David" == David P Grove <gr...@us...> writes: David> I think the high level question is do we really want BYTES_IN_PARTICLE != David> BYTES_IN_WORD? David> Under some object models, there is an advantage in 64 bit land to be able David> to "mis-align" the lowest word of the object so that a subsequent field David> (eg at offset 4 in the object) can be 64bit aligned. I think it is likely David> that with the switch to the forward scalar object model and the current David> placement of the array length to the "right" of the TIB, that there is no David> longer an advantage in doing this. David> If it makes the code simpler/cleaner than my suggestion would be that we David> drop the distinction between BYTES_IN_PARTICLE and BYTES_IN_WORD. David> There are still cases where the VM is going to want to allocate storage David> that (at a known offset from the start of the allocated region) is David> allocated to a larger unit than BYTES_IN_WORD (for example double[] in 32 David> bit mode and scalar objects that contain long/double fields in 32 bit David> mode). So, we need this ability, but subword alignment in 64 bit mode may David> not be that useful (at least under the current object model). Let me add a bit here .... Because some header fields become 64 bits long in the 64 bit world ... and because they sometimes need to be updated atomically ... and because the atomic operations require 64 bit alignment ... for PPC 64 bit, BYTES_IN_PARTICLE needs to be 8. Now, it is conceivable that a header design might start at an offset other than 0 within the 8 bytes. I would say that the offset within the particle is a separate issue, and a fully capable model would specify both alignment and offset: in effect a mask and value on some number of low order bits. (One could view this as a smaller particle size and a different kind of alignment constraint, I suppose.) Depending on the platform, the requirement I gave above might be relaxable: BYTES_IN_WORD might be 8 and BYTES_IN_PARTICLE 4 -- though in most cases such a choice is undesirable since it can lead to splitting across cache lines and a consequent drop in performance, etc. BYTES_IN_INT is always 4, because that's given by the language semantics; it serves only to document why one is using the number 4. Does any of this help? -- Eliot |
From: Steve B. <Ste...@an...> - 2005-01-26 01:25:05
|
>Because some header fields become 64 bits long in the 64 bit world ... and >because they sometimes need to be updated atomically ... and >because the atomic operations require 64 bit alignment ... > >for PPC 64 bit, BYTES_IN_PARTICLE needs to be 8. Now, it is conceivable >that a header design might start at an offset other than 0 within the 8 >bytes. I would say that the offset within the particle is a separate issue, >and a fully capable model would specify both alignment and offset: in >effect a mask and value on some number of low order bits. (One could view >this as a smaller particle size and a different kind of alignment >constraint, I suppose.) > Right. MMTk supports all of this and this is in fact what we're doing. BYTES_IN_PARTICLE is 4 and objects are 8 byte aligned internally. MMTk's allocation routines take a size, an alignment, and an offset which allow this very general sort of aligned allocation. >BYTES_IN_INT is always 4, because that's given by the language semantics; >it serves only to document why one is using the number 4. > But this is a problem, and one I have been trying to address. The above is true of Java, but not of other languages. MMTk tries to be neutral wrt to the supported language (obviously we're stuck with java as the implementation language). It was for exactly this reason that I expunged BYTES_IN_INT from Memory.java.... ....and in doing so violated some unwritten assumptions and triggered this lengthy thread. On a related note, I suggest we rename PARTICLE to be MINIMUM_ALIGNMENT. As far as I can tell, this is what it is. I think this name makes more sense as it is informative and it has symmetry with our existing MAXIMUM_ALIGNMENT. Unless there are objections I will go ahead and make this change soon. --Steve |
From: David P G. <gr...@us...> - 2005-01-26 01:40:35
|
> On a related note, I suggest we rename PARTICLE to be > MINIMUM_ALIGNMENT. As far as I can tell, this is what it is. I think > this name makes more sense as it is informative and it has symmetry with > our existing MAXIMUM_ALIGNMENT. Unless there are objections I will go > ahead and make this change soon. Sounds good. My wild guess is that if you replaced places in MMTk where it says BYTES_IN_INT with MINIMUM_ALIGNMENT, then things will be happy. One issue is that we need a mapping from MINIMUM_ALIGNMENT to the org.vmmagic.Address.load<FOO> and .store<FOO> functions that one should use to manipulate things of MINIMUM_ALIGNMENT bytes. I think this is one of the places where loadInt() is currently being used. Not sure what the cleanest way to do this is, which is partially what motivated my cop-out suggestion of merging MINIMUM_ALIGNMENT and BYTES_IN_WORD. I don't really want to do that, but if we are going to support them not being equal, then we need a clean way to specify the load/store instructions one would use.... --dave |
From: Steve B. <Ste...@an...> - 2005-01-26 01:46:20
|
> One issue is that we need a mapping from MINIMUM_ALIGNMENT to the > org.vmmagic.Address.load<FOO> and .store<FOO> functions that one should > use to manipulate things of MINIMUM_ALIGNMENT bytes. I think this is > one of the places where loadInt() is currently being used. Exactly. This is a problem in Memory.java because it does exactly this... More precisely, it zeros/sets memory in integer units... I haven't yet come up with a solution that I like ;-) --Steve |
From: Steve B. <Ste...@an...> - 2005-01-21 02:03:20
|
Sorry Kris, my fault. Thanks for identifying this so promptly. I am trying to expunge MMTk of Java-specific types (such as int). I will fix your immediate problem and then think harder about the solution. I didn't foresee this dependency, which just confirms my belief that this code needs cleaning up. --Steve |
From: Steve B. <Ste...@an...> - 2005-01-21 06:07:22
|
Kris, I believe I have fixed the immediate problems you raised. If you could check on a 64 bit platform that would be great. This all highlights a problem with undocumented assumptions. I think the changes I made were quite reasonable in the absence of any documentation to the contrary. Both pieces of code were virtually undocumented and included unobvious assumptions. I have done a major re-write/cleanup of Memory.java (including the addition of comments). I don't understand why it is necessary for Memory.java to work at the granularity of Java integers. It seems to me to be an artifact of implemenation, rather than intrinsic. If there is a very good reason, then we should add that explanation to the documentation. If there is not, then we should fix the code to use word-granularity operations. --Steve |
From: Eliot M. <mo...@cs...> - 2005-01-21 06:13:16
|
>>>>> "Steve" == Steve Blackburn <Ste...@an...> writes: Steve> I believe I have fixed the immediate problems you raised. If Steve> you could check on a 64 bit platform that would be great. Steve> This all highlights a problem with undocumented assumptions. I Steve> think the changes I made were quite reasonable in the absence of Steve> any documentation to the contrary. Both pieces of code were Steve> virtually undocumented and included unobvious assumptions. I Steve> have done a major re-write/cleanup of Memory.java (including the Steve> addition of comments). Steve> I don't understand why it is necessary for Memory.java to work Steve> at the granularity of Java integers. It seems to me to be an Steve> artifact of implemenation, rather than intrinsic. If there is a Steve> very good reason, then we should add that explanation to the Steve> documentation. If there is not, then we should fix the code to Steve> use word-granularity operations. Probably could be word granularity, if you're talking about bulk zero and bulk copy (and you assume word alignment of objects in 64-bit -- not obvious; in fact, maybe that's part of the issue here). Best wishes -- E |
From: Steve B. <Ste...@an...> - 2005-01-21 06:17:54
|
> Probably could be word granularity, if you're talking about bulk zero and > bulk copy (and you assume word alignment of objects in 64-bit -- not > obvious; in fact, maybe that's part of the issue here). This is exactly the assumption I made in my original cleanup, but apparently it killed things for 64 bit.... Not really sure why. If you can help with this, Kris, that would be great. BTW, the comment about lack of documentation was not a reflection on Kris or anyone else, but a lament on the state of some of the older MMTK & Jikes RVM code. --Steve |
From: David P G. <gr...@us...> - 2005-01-21 15:46:20
|
prototype is passing regression tests for me on PPC/AIX/64, so it looks like things are functional for now. Related observation. My guess is that most of Jikes RVM's VM_Memory.java is dead code and should perhaps be cleaned out? The arraycopy stuff in VM_Memory we are still using, but my guess is that most (all?) of the memory management stuff in there is dead? --dave |
From: Kris V. <Kris.Venstermans@UGent.be> - 2005-01-21 14:22:37
|
>> Probably could be word granularity, if you're talking about bulk zero >and >> bulk copy (and you assume word alignment of objects in 64-bit -- not >> obvious; in fact, maybe that's part of the issue here). >This is exactly the assumption I made in my original cleanup, but >apparently it >killed things for 64 bit.... Not really sure why. If you can help >with this, >Kris, that would be great. >BTW, the comment about lack of documentation was not a reflection on >Kris or >anyone else, but a lament on the state of some of the older MMTK & >Jikes RVM code. >--Steve This assumption is exactly the problem. Things do not need to be word-alignment. I believe BYTES_IN_PARTICLE defines the number of bytes on which objects needs to be aligned. I guess MMTk should be general enough to take into account that BYTES_IN_PARTICLE can be different then BYTES_IN_WORD. In Memory.java, you probaly can do something like in rvm's arraycopy(). If the start address is not word-aligned, first do one loadInt(). Then do loadWord(). In the end you migth need to do a loadInt() again. I know , it's ugly. Kris Venstermans |
From: Steve B. <Ste...@an...> - 2005-01-22 06:13:55
|
> This assumption is exactly the problem. That's right. This was clear from the begining. > word-alignment. I believe BYTES_IN_PARTICLE defines the number of bytes > on which objects needs to be aligned. > I guess MMTk should be general enough to take into account that > BYTES_IN_PARTICLE can be different then BYTES_IN_WORD. MMTk is general enough to take this into account. The problem arises when a) BYTES_IN_INT is used in place of BYTES_IN_PARTICLE, and b) when assumptions about alignment are not documented. In your original post you said that the code should revert to how it was (using BYTES_IN_INT). I don't think this is correct. Apparently it should be BYTES_IN_PARTICLE. I would like to ask those who did the 64 bit port to try to precisely identify what the alignment assumptions and dependencies, so that the code can be properly documented and uses of BYTES_IN_INT can be replaced with BYTES_IN_PARTICLE where appropriate. --Steve |