Skip to content

Split TBAA for array buffers #21262

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

Merged
merged 1 commit into from
Apr 6, 2017
Merged

Split TBAA for array buffers #21262

merged 1 commit into from
Apr 6, 2017

Conversation

Keno
Copy link
Member

@Keno Keno commented Apr 3, 2017

Array buffers with boxed and unboxed data do not alias. Expose this info to LLVM.

@Keno Keno added compiler:codegen Generation of LLVM IR and native code performance Must go faster labels Apr 3, 2017
@@ -2827,7 +2828,8 @@ static bool emit_builtin_call(jl_cgval_t *ret, jl_value_t *f, jl_value_t **args,
*ret = ghostValue(ety);
}
else {
*ret = typed_load(emit_arrayptr(ary, args[1], ctx), idx, ety, ctx, tbaa_arraybuf);
*ret = typed_load(emit_arrayptr(ary, args[1], ctx), idx, ety, ctx,
Copy link
Contributor

Choose a reason for hiding this comment

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

seems that you can have a bool isboxed a few lines above for this one too.

Copy link
Contributor

Choose a reason for hiding this comment

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

did not get addressed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just a small cleanup, will get it in separately.

@ararslan
Copy link
Member

ararslan commented Apr 3, 2017

@nanosoldier runbenchmarks(ALL, vs=":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels

@Keno
Copy link
Member Author

Keno commented Apr 3, 2017

Well, those performance results are moderately disappointing. I'll take a look.

@Keno
Copy link
Member Author

Keno commented Apr 6, 2017

I'm not able to reproduce these performance regressions:
@nanosoldier runbenchmarks(ALL, vs=":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels

@Keno
Copy link
Member Author

Keno commented Apr 6, 2017

Nanosoldier results look like noise. Few of them are the same between the two runs, and one that is got faster in the second run. Merging.

@Keno Keno merged commit 47c8fea into master Apr 6, 2017
@ararslan ararslan deleted the kf/splittbaa branch April 6, 2017 18:58
vtjnash added a commit that referenced this pull request Jun 18, 2019
This avoids a regression (correctness and performance) caused by #21262
where we were no longer able to fold away the isnull assertion
if there exists an explicit isassigned check.

Also upgrade many CreateInBoundsGEP calls to include the element type (NFC).
vtjnash added a commit that referenced this pull request Jun 18, 2019
This avoids a regression (correctness and performance) caused by #21262
where we were no longer able to fold away the isnull assertion in the
case where the source also contains an explicit isassigned check.

Also upgrade many CreateInBoundsGEP calls to include the element type (NFC).
vtjnash added a commit that referenced this pull request Jun 26, 2019
This avoids a regression (correctness and performance) caused by #21262
where we were no longer able to fold away the isnull assertion in the
case where the source also contains an explicit isassigned check.

Also upgrade many other CreateInBoundsGEP calls to include the element type (NFC).
KristofferC pushed a commit that referenced this pull request Jul 1, 2019
This avoids a regression (correctness and performance) caused by #21262
where we were no longer able to fold away the isnull assertion in the
case where the source also contains an explicit isassigned check.

Also upgrade many other CreateInBoundsGEP calls to include the element type (NFC).

(cherry picked from commit a7427aa)
KristofferC pushed a commit that referenced this pull request Aug 26, 2019
This avoids a regression (correctness and performance) caused by #21262
where we were no longer able to fold away the isnull assertion in the
case where the source also contains an explicit isassigned check.

Also upgrade many other CreateInBoundsGEP calls to include the element type (NFC).

(cherry picked from commit a7427aa)
KristofferC pushed a commit that referenced this pull request Aug 26, 2019
This avoids a regression (correctness and performance) caused by #21262
where we were no longer able to fold away the isnull assertion in the
case where the source also contains an explicit isassigned check.

Also upgrade many other CreateInBoundsGEP calls to include the element type (NFC).

(cherry picked from commit a7427aa)
KristofferC pushed a commit that referenced this pull request Feb 20, 2020
This avoids a regression (correctness and performance) caused by #21262
where we were no longer able to fold away the isnull assertion in the
case where the source also contains an explicit isassigned check.

Also upgrade many other CreateInBoundsGEP calls to include the element type (NFC).

(cherry picked from commit a7427aa)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:codegen Generation of LLVM IR and native code performance Must go faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants