Skip to content

remove IRState.falseBlock#14943

Closed
WalterBright wants to merge 1 commit intodlang:masterfrom
WalterBright:falseCTFEblocks
Closed

remove IRState.falseBlock#14943
WalterBright wants to merge 1 commit intodlang:masterfrom
WalterBright:falseCTFEblocks

Conversation

@WalterBright
Copy link
Member

Shouldn't need it anymore.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @WalterBright!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#14943"

Comment on lines +155 to +156
if (!s.isIfCtfeBlock()) // __ctfe is always false at runtime
Statement_toIR(s.ifbody, irs, &mystate);
Copy link
Member

Choose a reason for hiding this comment

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

Isn't the inverse needed for the else block when if (!__ctfe) ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, if we really wanted to support that. I'm on the fence about it. There are other __ctfe constructs, too. I figure we make this solid first.

Copy link
Member

@ibuclaw ibuclaw left a comment

Choose a reason for hiding this comment

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

This should target stable, as it's a continuation of the fix for regression 23710 in all but name.

@WalterBright
Copy link
Member Author

macOS 13.x x64 (M1), DMD (latest) Failing after 22m

Looks like a problem with master, not this PR. @RazvanN7 ?

@ibuclaw
Copy link
Member

ibuclaw commented Mar 2, 2023

macOS 13.x x64 (M1), DMD (latest) Failing after 22m

Looks like a problem with master, not this PR. @RazvanN7 ?

Seems like dmd's yl2xp1 intrinsic doesn't work on macOS 13.x (emulating x86 on M1) with small inputs.

log1p(0x1p-16382): expected: 0x1p-16382 | got: 0x0p+0
****** FAIL release64 std.math.exponential
core.exception.AssertError@std/math/exponential.d(3765): Assertion failure
----------------

fabs(0x1p-16382) = 3.3621e-4932
fabs(0x1p-16382) <= 0.25 = true
yl2xp1(0x1p-16382) = 0x0p+0

@WalterBright
Copy link
Member Author

@ibuclaw I made this for stable #14946 because my dev work on master needs this one.

@ibuclaw
Copy link
Member

ibuclaw commented Mar 3, 2023

Shouldn't be required to have both, just merge stable then create a PR to git merge stable --no-ff on master.

@WalterBright
Copy link
Member Author

Every time I've tried something like that I get a PR with ten thousand lines of difference. Just merge both PRs and things will be fine!

@ibuclaw
Copy link
Member

ibuclaw commented Mar 3, 2023

Automated merging of stable fixes into master is something that needs looking into as it's quite silly to have bug fixes diverged from mainline for any lengthy period of time.

@WalterBright
Copy link
Member Author

If git had a git undo command I'd be a lot more adventurous with it. But as it stands, it's easier to just delete 16 lines into a new PR and submit it.

@ibuclaw
Copy link
Member

ibuclaw commented Mar 3, 2023

Can't do anything about Apple having buggy emulators except turn off fyl2x instructions on macOS.

dlang/phobos#8705

@WalterBright
Copy link
Member Author

Thanks for identifying the problem and supplying a workaround.

@ibuclaw
Copy link
Member

ibuclaw commented Mar 3, 2023

Superseded by #14947.

@ibuclaw
Copy link
Member

ibuclaw commented Mar 3, 2023

If git had a git undo command I'd be a lot more adventurous with it. But as it stands, it's easier to just delete 16 lines into a new PR and submit it.

Every change you make with git goes into the reflog, you don't lose it. https://git-scm.com/docs/git-reflog

git reflog

Which lists both commit hashes which can also be referenced with HEAD@{N}.

Undo then becomes:

git reset --hard the-reflog-hash

Or

git reset --hard HEAD@{1}

@ibuclaw ibuclaw closed this Mar 3, 2023
@ibuclaw
Copy link
Member

ibuclaw commented Mar 3, 2023

Other PR merged.

@WalterBright
Copy link
Member Author

Thanks for reflog tip

@WalterBright WalterBright deleted the falseCTFEblocks branch March 3, 2023 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants