Skip to content

AMD64 LIR: Lower constant add/sub to lea #263

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Nov 17, 2017
Merged

AMD64 LIR: Lower constant add/sub to lea #263

merged 6 commits into from
Nov 17, 2017

Conversation

@jcdavis
Copy link
Contributor

@jcdavis jcdavis commented Nov 8, 2017 •
edited
Loading

This is a cleaned up but more limited version of my previous commit. It considers only add/sub of 32 bit constants besides 1 as candidates, which means in the "worst" case (where the source and dest reg are the same), it is still the same length instruction the equivalent add/sub, and in the best case is 1 mov shorter

I get 1 unit test failure locally, but it seems unrelated:

Time: 277.556
There was 1 failure:
1) testWriteToObjectWithValueProperty(com.oracle.truffle.sl.test.SLTckTest)
java.lang.IllegalStateException: Language environment is already disposed.
        at com.oracle.truffle.truffle_api/com.oracle.truffle.api.TruffleLanguage$Env.checkDisposed(TruffleLanguage.java:1077)
        at com.oracle.truffle.truffle_api/com.oracle.truffle.api.TruffleLanguage$Env.in(TruffleLanguage.java:1338)
        at com.oracle.truffle.sl.runtime.SLContext.<init>(SLContext.java:109)
        at com.oracle.truffle.sl.SLLanguage.createContext(SLLanguage.java:85)
        at com.oracle.truffle.sl.SLLanguage.createContext(SLLanguage.java:70)
        at com.oracle.truffle.truffle_api/com.oracle.truffle.api.TruffleLanguage$LanguageImpl.createEnvContext(TruffleLanguage.java:1710)
        at com.oracle.truffle.truffle_api/com.oracle.truffle.api.vm.PolyglotEngine$Language.getEnv(PolyglotEngine.java:1476)
        at com.oracle.truffle.truffle_api/com.oracle.truffle.api.vm.PolyglotEngine$LegacyEngineImpl.getEnvForInstrument(PolyglotEngine.java:1555)
        at com.oracle.truffle.tck.TruffleTCK.assertIsObjectOfLanguage(TruffleTCK.java:2330)
        at com.oracle.truffle.tck.TruffleTCK.testWriteToObjectWithValueProperty(TruffleTCK.java:1864)

@graalvmbot
Copy link
Collaborator

If you believe this is an error, please contact graal-dev@openjdk.java.net.

@jcdavis jcdavis changed the title x86 LIR: Lower constant add/sub to lea AMD64 LIR: Lower constant add/sub to lea Nov 8, 2017
gilles-duboscq
gilles-duboscq reviewed Nov 10, 2017
View reviewed changes
...lvm.compiler.core.amd64/src/org/graalvm/compiler/core/amd64/AMD64ArithmeticLIRGenerator.java
boolean isAvx = ((AMD64) target.arch).getFeatures().contains(CPUFeature.AVX);
switch ((AMD64Kind) a.getPlatformKind()) {
case DWORD:
if (isJavaConstant(b) && !setFlags) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is repeated 4 times, i think we could factor it out.

@jcdavis
Refactor out common bits and remove cases from sub
1ade134 
SubNode will canonicalize substracting a constant into an AddNode of
its complement. Therefore, we can skip attempting to handle that case
(And therefore the tricky corner case of Integer.MIN_VALUE).
@gilles-duboscq
Copy link
Member

Seems fine to me.
Please tell us once you've taken care of the OCA.

@jcdavis
Copy link
Contributor Author

jcdavis commented Nov 14, 2017 •
edited
Loading

Done :): http://www.oracle.com/technetwork/community/oca-486395.html#d

@graalvmbot
Copy link
Collaborator

  • User Jackson Davis with email address jackson -(at)- jcdav -(dot)- is is now cleared for contributions.

@dougxc dougxc merged commit 1ade134 into oracle:master Nov 17, 2017
@jcdavis jcdavis deleted the lea-add-sub branch November 19, 2017 16:50
@dougxc dougxc added the accept label Dec 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gilles-duboscq gilles-duboscq gilles-duboscq left review comments

Assignees

No one assigned

Labels

accept oca-signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jcdavis @graalvmbot @gilles-duboscq @dougxc