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

Fix optimization of CAST(int)(LSH(long)<<CNS_INT 32+ with side effects).#19899

Merged
sandreenko merged 2 commits intodotnet:masterfrom
sandreenko:GitHub_19272
Sep 12, 2018
Merged

Fix optimization of CAST(int)(LSH(long)<<CNS_INT 32+ with side effects).#19899
sandreenko merged 2 commits intodotnet:masterfrom
sandreenko:GitHub_19272

Conversation

@sandreenko
Copy link

@sandreenko sandreenko commented Sep 11, 2018

Fix #19272.
Thanks @jakobbotsch for the issue and the repro.

@sandreenko
Copy link
Author

sandreenko commented Sep 11, 2018 •
edited
Loading

@mmitche could you please take a look at Ubuntu x64 job? It fails with Disk has run out of space and cannot get latest changes. Please contact netciadmin@microsoft.com. IOException: No space left on device (example).

@mmitche
Copy link
Member

mmitche commented Sep 11, 2018

@safern Got a link? I don't see the failure. Machine can usually just be deleted.

jakobbotsch
jakobbotsch reviewed Sep 11, 2018
View reviewed changes
src/jit/morph.cpp Outdated
const ssize_t shiftAmountValue = shiftAmount->AsIntCon()->IconValue();

if (shiftAmountValue >= 64)
if ((shiftAmountValue >= 64) && (shiftAmountValue < 0))
Copy link
Member

@jakobbotsch jakobbotsch Sep 11, 2018

Choose a reason for hiding this comment

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

This should be || I suppose?

Copy link
Author

@sandreenko sandreenko Sep 11, 2018

Choose a reason for hiding this comment

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

Yes, of course. Thank you.

@sandreenko
Copy link
Author

sandreenko commented Sep 11, 2018

@safern Got a link? I don't see the failure. Machine can usually just be deleted.

Added an example link.

@sandreenko
Copy link
Author

sandreenko commented Sep 11, 2018

PTAL @AndyAyersMS @dotnet/jit-contrib

CarolEidt
CarolEidt approved these changes Sep 11, 2018
View reviewed changes
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

@sandreenko
Copy link
Author

sandreenko commented Sep 11, 2018

@dotnet-bot test this please

AndyAyersMS
AndyAyersMS approved these changes Sep 12, 2018
View reviewed changes
Copy link
Member

@AndyAyersMS AndyAyersMS left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this.

@sandreenko
Copy link
Author

sandreenko commented Sep 12, 2018

Hm, I see only 13 jobs were run for this job (3 pending and 10 successful checks ), lets try to restart.

@sandreenko sandreenko closed this Sep 12, 2018
@sandreenko sandreenko reopened this Sep 12, 2018
@sandreenko sandreenko merged commit a3443a5 into dotnet:master Sep 12, 2018
@sandreenko sandreenko deleted the GitHub_19272 branch September 12, 2018 03:32
This was referenced Feb 2, 2021
Merged
Closed
Merged
@briansull briansull mentioned this pull request Feb 12, 2021
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

4 more reviewers

@jakobbotsch jakobbotsch jakobbotsch left review comments

@CarolEidt CarolEidt CarolEidt approved these changes

@AndyAyersMS AndyAyersMS AndyAyersMS approved these changes

@briansull briansull briansull approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-CodeGen

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@sandreenko @mmitche @CarolEidt @jakobbotsch @AndyAyersMS @briansull