Skip to content
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.

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

Simplify TempVector Offset from IR #4857

Merged

Conversation

@MarcusDenker
Copy link
Member

MarcusDenker commented Oct 7, 2019

simplify getting the index from the temp vector from the IR

  • make sure that we ask the right scope
  • simplify the code on the IR Level
  - make sure that we ask the right scope
  - simplify the code on the IR Level
@MarcusDenker

This comment has been minimized.

Copy link
Member Author

MarcusDenker commented Oct 8, 2019

The idea behind this change: #indexInTempVectorFromIR: was always asking the method scope. This means that # tempVectorNamed: there was implemented to search the whole IR tree for the temp vector.

Now we instead get first the definition scope of the vector (using #tempVectorDefinitionScopeForTempStoringIt) and then ask that scope, There we can be 100% sure that a temp vector is defined in the IR sequence.

This in turn then allows us to radically simplify tempVectorNamed: to just call it on the sequence.

Lots of checking code can be removed (e.g. hasTempVector:, #blockHasTempVector: and so on).

The step after this is to simplify tempVectorDefinitionScopeForTempStoringIt, for that we have issue #4843

(instead of calling #tempVectorDefinitionScopeForTempStoringIt we then can just do "originalVar scope")

@MarcusDenker MarcusDenker merged commit c5a5933 into pharo-project:Pharo8.0 Oct 9, 2019
1 of 4 checks passed
1 of 4 checks passed
continuous-integration/benchmarks The benchmarks show regressions.
Details
continuous-integration/jenkins/pr-merge This commit has test failures
Details
probot/minimum-reviews Pending review approvals
WIP Ready for review
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.