Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Clear GTF_CALL from non-calls in rationalize #11917

Merged
merged 1 commit into from
May 26, 2017

Conversation

JosephTremoulet
Copy link

Since rationalization hoists comma antecedents out into separate
statements, any side-effects that the antecedent had might no longer apply
to the comma's ancestor nodes. Moreover, in LIR, side-effect flags need
only describe the immediate node; summary flags are computed when Ranges
are constructed. So, update rationalize to clear the GTF_CALL flag from
any node that is not a call. Failure to do so gives an
overly-conservative view of side-effects, which can inhibit dead store
removal.

Since rationalization hoists comma antecedents out into separate
statements, any side-effects that the antecedent had might no longer apply
to the comma's ancestor nodes.  Moreover, in LIR, side-effect flags need
only describe the immediate node; summary flags are computed when Ranges
are constructed.  So, update rationalize to clear the `GTF_CALL` flag from
any node that is not a call.  Failure to do so gives an
overly-conservative view of side-effects, which can inhibit dead store
removal.
@JosephTremoulet
Copy link
Author

@pgavlin PTAL; this implements your suggestion from #10946.
/cc @dotnet/jit-contrib

jit-diff release --frameworks --tests shows just one diff, improved deadcode elimination:

Found files with textual diffs.
Summary:
(Note: Lower is better)
Total bytes of diff: -4 (0.00 % of base)
    diff is an improvement.
Total byte diff includes 0 bytes from reconciling methods
        Base had    0 unique methods,        0 unique bytes
        Diff had    0 unique methods,        0 unique bytes
Top file improvements by size (bytes):
          -4 : JIT\IL_Conformance\Old\Conformance_Base\starg_i\starg_i.dasm (-10.00 % of base)
1 total files with size differences (1 improved, 0 regressed), 6513 unchanged.
Top method improvements by size (bytes):
          -4 : JIT\IL_Conformance\Old\Conformance_Base\starg_i\starg_i.dasm - _starg:main(ref):int
1 total methods with size differences (1 improved, 0 regressed), 314382 unchanged.

Copy link

@pgavlin pgavlin left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for doing this!

Copy link

@CarolEidt CarolEidt left a comment

Choose a reason for hiding this comment

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

LGTM

@JosephTremoulet JosephTremoulet merged commit 74c988e into dotnet:master May 26, 2017
@JosephTremoulet JosephTremoulet deleted the RationalCall branch May 26, 2017 05:41
@karelz karelz modified the milestone: 2.1.0 Aug 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants