RFR: 8013393: Merge templateTable_x86 32 and 64 bit files into one file.
John Rose
john.r.rose at oracle.com
Mon Mar 2 21:45:16 UTC 2015
On Mar 2, 2015, at 11:46 AM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
>
> That’s quite a large change (I wish we had Crucible or something to leave inline comments).
>
> First, you have to keep the oldest copyright year (but maybe that’s just the diff).
>
> + #ifdef _LP64
> + void TemplateTable::lconst(int value) {
>
> I’d prefer to have #ifdef’s inside methods and not around them. There are a bunch; please change all of them.
I agree Christian's comments, and especially with this request.
A few more points on small matters:
In our source base, C++ comments usually begin with two slashes and a space (less than 10% exceptions).
+//Global Register Names
+//Address Computation: local variables
(etc.)
Let's use the term "amd64" only where required; x86_64 is the standard now.
// No amd64 specific initialization
More on #ifdefs: I think there are too many; Chris has pointed out the main problem.
It's a judgement call, but here's another example:
+#ifdef _LP64
+ Register rtmp = r8;
+ Register rthread = r15_thread;
+#else
+ Register rtmp = rsi;
+ Register rthread = rcx;
+ __ get_thread(rcx);
+ __ save_bcp();
+#endif // _LP64
could just as well be:
+ Register rtmp = LP64_ONLY(r8) NOT_LP64(rsi);
+ Register rthread = LP64_ONLY(r15_thread) NOT_LP64(rcx);
+ LP64_ONLY(__ get_thread(rcx));
+ LP64_ONLY(__ save_bcp());
Advantages: Fewer lines, less duplication, less chance of merge problems.
Possible objection: Ugly; but only in a different way from the other ugly.
However, I admit that is a marginal example.
— John
More information about the hotspot-dev
mailing list