RFR: Add support of lzcnt and tzcnt

[Igor Veresov]

Hm, that’s odd. It built and ran just fine for me (gate and build). The change contains the following (com.oracle.graal.hotspot depends on com.oracle.graal.replacements already) to be able to access HotSpotGraalRuntime for c.o.g.replacements.amd64:

diff --git a/mx/suite.py b/mx/suite.py
--- a/mx/suite.py
+++ b/mx/suite.py
@@ -369,7 +369,6 @@
       "sourceDirs" : ["src"],
       "dependencies" : [
         "com.oracle.graal.compiler.amd64",
-        "com.oracle.graal.hotspot",
         "com.oracle.graal.replacements.amd64",
       ],
       "checkstyle" : "com.oracle.graal.graph",
@@ -632,7 +631,9 @@
     "com.oracle.graal.replacements.amd64" : {
       "subDir" : "graal",
       "sourceDirs" : ["src"],
-      "dependencies" : ["com.oracle.graal.replacements"],
+      "dependencies" : [
+          "com.oracle.graal.hotspot"
+          ],
       "checkstyle" : "com.oracle.graal.graph",
       "javaCompliance" : "1.8",
       "annotationProcessors" : ["com.oracle.graal.service.processor”],


Just did a clean test, checked out basic-graal, applied the patch (basic-graal.patch <http://cr.openjdk.java.net/~iveresov/lztzcnt/webrev.01/basic-graal.patch>) - all seems to build ok.

igor

> On Nov 17, 2014, at 5:30 AM, Gilles Duboscq <duboscq at ssw.jku.at> wrote:
> 
> Hello Igor,
> 
> I tried integrating your changes but there is a dependency problem:
> In AMD64Guards, which is an amd64-specific project, you are trying to
> access HotSpotGraalRuntime which lives in a hotspot-specific project.
> That's not possible. It shouldn't even have built.
> How did you build it?
> 
> I suppose you'd need a better substitution guard system, which could
> provide you with the Architecture object.
> 
> -Gilles
> 
> On Sun, Nov 16, 2014 at 9:56 AM, Igor Veresov <igor.veresov at oracle.com <mailto:igor.veresov at oracle.com>> wrote:
>> Hi Gilles,
>> 
>> Thanks for the review!
>> Please find the replies inline.
>> 
>>> On Nov 15, 2014, at 4:06 AM, Gilles Duboscq <duboscq at ssw.jku.at> wrote:
>>> 
>>> Hi Igor,
>>> 
>>> -BitScanForwardNode.inferStamp:
>>> looks like firstMaybeSetBit and min can now be set outside of the if/else
>> 
>> Good point. Fixed.
>> 
>>> -BitOpsTest:
>>> You could have used UnsafeAccess
>>> 
>> Aha, thanks for pointing that out. I grepped for something like it but didn’t notice UnsafeAccess. Fixed.
>> 
>>> The rest looks good to me. Thanks.
>> 
>> Updated webrev: http://cr.openjdk.java.net/~iveresov/lztzcnt/webrev.01/
>> 
>>> 
>>> Overall, it looks like we should rather use the Count*ZerosNode at the
>>> high level regardless of the platform and lower them in a
>>> platform-dependent way. On AMD64 without lzcnt/tzcnt we would then
>>> introduce the BitScan* nodes only during platform-dependent lowering.
>>> We should put that on the todo-list if we introduce a proper
>>> platform-specific lowering phase before LIR.
>> 
>> It’s a possible approach, I liked substitution guards though. They are fairly expressive.  May be we can allow a form of substitution specialization, in which there is a generic implementation and then we can define a specialized version for some platform that will automatically replace it. I guess it may be achieved, for example, by prioritizing replacement providers.
> 
> Yes, my thinking was that the Cound*ZerosNode are in general more
> high-level and simpler to reason about.
> 
>> 
>> igor
>> 
>> 
>>> 
>>> -Gilles
>>> 
>>> On Sat, Nov 15, 2014 at 8:30 AM, Igor Veresov <igor.veresov at oracle.com> wrote:
>>>> The change adds the following:
>>>> - support of lzcnt and tzcnt instructions,
>>>> - unit tests for lzcnt/tzcnt,
>>>> - ability to emit bsf/bsr in case lzcnt/tzcnt were turned off from the command line,
>>>> - tightening the stamps produced by ScanBitForward/ReverseNode nodes.
>>>> 
>>>> 
>>>> Webrev: http://cr.openjdk.java.net/~iveresov/lztzcnt/webrev.00/
>>>> 
>>>> Thanks!
>>>> igor