RFR(L): 8031321 Support Intel bit manipulation instructions
Igor Veresov
igor.veresov at oracle.com
Fri Feb 7 23:54:01 PST 2014
Thanks!
igor
On Feb 7, 2014, at 8:31 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
> Good.
>
> Thanks,
> Vladimir
>
> On 2/7/14 7:57 PM, Igor Veresov wrote:
>>
>> On Feb 7, 2014, at 6:41 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>
>>> You added 3 %s. Did we miss one before?
>>
>> Yup.
>>
>>> You did not updated last #if in matcher.cpp where you clone node.
>>
>> Yes. Right. Forgot about that instance.
>>
>>
>>> And I don't see updated test - it still use -server.
>>>
>>
>> Sorry about that. Changed another copy of it… Fixed.
>>
>> New webrev: http://cr.openjdk.java.net/~iveresov/8031321/webrev.02/
>>
>> igor
>>
>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 2/7/14 6:19 PM, Igor Veresov wrote:
>>>> Thanks for the review, Vladimir!
>>>>
>>>> On Feb 7, 2014, at 3:53 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>>>
>>>>> assembler_x86.cpp
>>>>>
>>>>> InstructionMark is only used in instructions with Address argument.
>>>>>
>>>> Done.
>>>>
>>>>> It does not make sense to pass all arguments to new prefix functions. Most passed arguments are the same:
>>>>>
>>>>> dst, src1, src2, VEX_SIMD_NONE, VEX_OPCODE_0F_38, false, false
>>>>> dst, src1, src2, VEX_SIMD_NONE, VEX_OPCODE_0F_38, true, false
>>>>>
>>>>> I think you can declare new 32-bit functions using simd_prefix_ :
>>>>>
>>>>> void vex_prefix_0F38(Register dst, Register nds, Address src) {
>>>>> bool vex_w = false;
>>>>> bool vector256 = false;
>>>>> vex_prefix(src, nds->encoding(), dst->encoding(),
>>>>> VEX_SIMD_NONE, VEX_OPCODE_0F_38, vex_w, vector256);
>>>>> }
>>>>>
>>>>> and related 64-bit:
>>>>>
>>>>> void vex_prefix_0F38_q(Register dst, Register nds, Address src) {
>>>>> bool vex_w = true;
>>>>>
>>>>
>>>> Done.
>>>>
>>>>> New countTrailingZeros formats in both .ad files have unneeded "\n\t" at the end.
>>>>>
>>>> Done.
>>>>
>>>>>
>>>>> matcher.hpp, .cpp
>>>>> Why you did not use #ifdef X86?
>>>>>
>>>>
>>>> Good point. Done.
>>>>
>>>>> Next formula in the comment:
>>>>> (AndL (SubL (Con0 LoadL*) LoadL*))
>>>>> should be:
>>>>> (AndL (SubL Con0 LoadL*) LoadL*)
>>>>>
>>>>
>>>> Done.
>>>>
>>>>
>>>>> *_idx % 2 + 1 should be *_idx & 1 + 1
>>>>>
>>>>
>>>> Works either way. Done.
>>>>
>>>>>
>>>>> vm_version_x86.cpp - add bm1, bm2 support to _features_str (in jio_snprintf() output).
>>>>>
>>>>
>>>> Done.
>>>>
>>>>> The test should not use -server otherwise wrong VM will be tested.
>>>>>
>>>>
>>>> Done.
>>>>
>>>>
>>>> The updated webrev: http://cr.openjdk.java.net/~iveresov/8031321/webrev.01/
>>>>
>>>> Thanks!
>>>> igor
>>>>
>>>>
>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>>> On 2/6/14 2:10 PM, Igor Veresov wrote:
>>>>>> This change adds support for BMI1 instructions on x86 (supported on AMD Piledriver and Intel Haswell).
>>>>>> The changes in the matcher.cpp are kind of a temporary hack to workaround the inability to describe DAGs in ADL. I’ll address that problem properly a bit later (we need this change to land in 8u20).
>>>>>>
>>>>>> I’m also using Rickard’s changes in type.hpp that are not quite in the repo yet.
>>>>>>
>>>>>> Webrev: http://cr.openjdk.java.net/~iveresov/8031321/webrev.00/
>>>>>>
>>>>>> Testing: jprt, jtreg, ctw, the new regtest (verified that the instructions are generated and all).
>>>>>>
>>>>>> igor
>>>>>>
>>>>
>>
More information about the hotspot-compiler-dev
mailing list