RFR: 8241438: Move IntelJccErratum mitigation code to platform-specific code
Erik Österlund
erik.osterlund at oracle.com
Tue Mar 24 16:37:57 UTC 2020
Hi Vladimir,
Thank you for the review.
On 2020-03-23 21:18, Vladimir Kozlov wrote:
> Hi Erik
>
> Why you calling Compile::current() in PhaseOutput()?
> Base class Phase has C value set already.
Oops, I accidentally got that line from a merge conflict. My bad. Will
remove that line.
> I don't think you can use virtual method compute_padding() as you do.
> Call nodes and other have own versions.
I already in the current tip assert that there is not conflicting
paddings between what the node wants
what the JCC node wants. If there was such an issue, we would have
asserted by now. For example, Java calls
have padding due to cross-modifying code. But that same padding makes it
JCC-safe, so the JCC padding code
will not request a different padding.Therefore it is safe to do it this way.
> The same for alignment_required().
And same as I said above. ;)
> I would suggest instead to add PhaseOutput platform specific methods
> and pass Node* parameter.
Do you still suggest that after my explanations above that the
alignment/padding requrement is already
asserted to not be conflicting between JCC code and individual mach nodes?
> In pd_alignment_required() you implicitly use knowledge that
> relocInfo::addr_unit() on x86 is 1.
> At least add comment about that.
I can add a comment about that.
New webrev:
http://cr.openjdk.java.net/~eosterlund/8241438/webrev.01/
Incremental:
http://cr.openjdk.java.net/~eosterlund/8241438/webrev.00_01/
Thanks,
/Erik
> Thanks,
> Vladimir
>
> On 3/23/20 6:09 AM, Erik Österlund wrote:
>> Hi,
>>
>> There is some platform-specific code in PhaseOutput that deals with
>> the IntelJccErratum mitigation,
>> which is ifdef:ed in shared code. It should move to platform-specific
>> code.
>>
>> This patch exposes the iteration state of PhaseOutput, which allows
>> hiding the Intel-specific code
>> completely in x86-specific files.
>>
>> Webrev:
>> http://cr.openjdk.java.net/~eosterlund/8241438/webrev.00/
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8241438
>>
>> Thanks,
>> /Erik
More information about the hotspot-compiler-dev
mailing list