Request for review(XL): 6985015: C1 needs to support compressed oops

[John Coomes]

Igor Veresov (igor.veresov at oracle.com) wrote:
> On 11/19/10 4:37 PM, John Coomes wrote:
> > Igor Veresov (igor.veresov at oracle.com) wrote:
> >> On 11/19/10 2:26 PM, Igor Veresov wrote:
> >>
> >>>
> >>>> src/cpu/x86/vm/c1_LIRGenerator_x86.cpp:
> >>>>
> >>>> 1143 if (!x->klass()->is_loaded() || LP64_ONLY(UseCompressedOops)
> >>>> NOT_LP64(false)) {
> >>>> 1165 if (!x->klass()->is_loaded() || LP64_ONLY(UseCompressedOops)
> >>>> NOT_LP64(false)) {
> >>>> I think the common pattern is LP64_ONLY(|| UseCompressedOops).
> >>>
> >>> It is, but do you think it would be more readable to factor this
> >>> predicate to separate function. It just determines if we need an extra
> >>> temporary. I'd rather leave it as is.
> >>>
> >>
> >> Sorry, I didn't get what you meant at first. Ramki explained it to me.
> >> Thanks, I'll fix that.
> >
> > BTW, why do you need/want to guard UseCompressedOops with LP64_ONLY()?
> > UseCompressedOops is constant false in 32-bit.
> >
> 
> Hm, good question. It's actually a common pattern with compressed oops. 
> The only reason I can see is to make the jvmg version a bit faster. But 
> I would like not to do the global ifdef removal with this change, it's 
> already big enough.

I wasn't suggesting you include it in your changes; it it's going to
be done it should be done separately.  I'm more interested in finding
out why the pattern is used at all.

-John