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

Decompose some bitwise operations in HIR to allow more overall optimizations to kick in #104517

Merged
merged 14 commits into from
Jul 13, 2024

Conversation

tannergooding
Copy link
Member

@tannergooding tannergooding commented Jul 7, 2024

Doing this allows more scenarios involving CSE, morph transforms, and other optimizations to kick in. This will also longer term allow ~Compare to be optimized to the inverse comparison as well.

When such optimizations aren't feasible, we still end up getting the good codegen in lowering anyways.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 7, 2024
@tannergooding tannergooding marked this pull request as ready for review July 9, 2024 04:59
@tannergooding
Copy link
Member Author

CC. @dotnet/jit-contrib, this is ready for review.

It gives some nice improvements on both Arm64 and x64 due to additional optimization opportunities. On xarch in particular it also gives better codegen almost anywhere vpternlog can be used and handles more (but not all) of the possible vpternlog lightups.

There are a couple methods where the code size has increased, rather than decreased, due to vpternlog but its a minority overall and typically due to 2 of the inputs being containable. These should be addressable, but I'd rather do that separately since this is still a large net improvement (generally speaking this should entail checking if two operands are containable and opting to not combine them into a vpternlog in that scenario).

@tannergooding
Copy link
Member Author

The SPMI replay failure is #104585

@tannergooding
Copy link
Member Author

Rerunning now that the SPMI replay issue should be fixed. This should still be ready for review and notably also fixes some issues that were found by antigen

// We specially handle this here since we're only producing a
// native intrinsic node in LIR

std::swap(op1, op2);
Copy link
Member

Choose a reason for hiding this comment

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

can you remind me - is it fine to do this swap here in terms of side-effect reordering?

Copy link
Member

Choose a reason for hiding this comment

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

ah, it's LIR so I guess it is

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, in this case its fine specifically because we've validated we're in LIR already.

Otherwise, we should be applying GTF_REVERSE_OPS or have some comment about expecting the user to have already spilled side effects, etc.

Copy link
Member

@EgorBo EgorBo left a comment

Choose a reason for hiding this comment

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

Changes LGTM assuming CI failures are unrelated

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants