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

fix workaround which does not use target native features on Windows #508

Closed
andrewrk opened this issue Sep 30, 2017 · 9 comments
Closed
Labels
bug Observed behavior contradicts documented or intended behavior os-uefi os-windows standard library This issue involves writing Zig code for the standard library. upstream An issue with a third party project that Zig uses.
Milestone

Comments

@andrewrk
Copy link
Member

andrewrk commented Sep 30, 2017

See #302. I had to add a workaround to treat every build on windows as if we were cross compiling for the native target. Once the llvm bug is fixed we can remove this workaround.

See below.

mailing list message: http://lists.llvm.org/pipermail/llvm-dev/2017-September/117860.html

@andrewrk andrewrk added the bug Observed behavior contradicts documented or intended behavior label Sep 30, 2017
@andrewrk andrewrk added this to the 0.2.0 milestone Sep 30, 2017
andrewrk added a commit that referenced this issue Sep 30, 2017
when target native features are used.

See #508
andrewrk added a commit that referenced this issue Oct 3, 2017
I had to revert the target native features thing because there
is still some incorrect behavior with f128.

Reopens #508
partially reverts b505462

See #302
@andrewrk andrewrk reopened this Oct 3, 2017
@andrewrk andrewrk modified the milestone: 0.2.0 Oct 19, 2017
@andrewrk andrewrk modified the milestones: 0.2.0, 0.3.0 Jan 6, 2018
@andrewrk andrewrk modified the milestones: 0.3.0, 0.4.0 Feb 28, 2018
@andrewrk
Copy link
Member Author

Accident, incorrectly tagged this issue in the above commit.

@andrewrk andrewrk reopened this Feb 27, 2019
@andrewrk andrewrk added the standard library This issue involves writing Zig code for the standard library. label Mar 1, 2019
@suirad
Copy link
Contributor

suirad commented Mar 8, 2019

Is this not an upstream issue, since it depends on llvm changes?

@andrewrk
Copy link
Member Author

andrewrk commented Mar 8, 2019

I don't think it's an upstream bug anymore. I think it's a problem with Zig's compiler-rt ABI compliance on Windows. Not aligning enough or something like that.

@emekoi
Copy link
Contributor

emekoi commented May 7, 2019

can we add back native features thanks to #2079?

@andrewrk
Copy link
Member Author

can we add back native features thanks to #2079?

I'll run the tests and report back.

@andrewrk
Copy link
Member Author

Here's what I get:

394/686 behavior.math.test "behavior-windows-x86_64-Debug-bare-multi f128"...test failure
C:\msys64\home\andy\dev\zig\build-release-llvm8\lib\zig\std\testing.zig:149:14: 0x7ff6578a1029 in std.testing.expect (test.obj)
    if (!ok) @panic("test failure");
             ^
C:\msys64\home\andy\dev\zig\test\stage1\behavior\math.zig:538:11: 0x7ff6578b9e88 in behavior.math.test_f128 (test.obj)
    expect(make_f128(1.0) != 1.1);
          ^
C:\msys64\home\andy\dev\zig\test\stage1\behavior\math.zig:527:14: 0x7ff6578a926f in behavior.math.test "behavior-windows-x86_64-Debug-bare-multi f128" (test.obj)
    test_f128();
             ^
C:\msys64\home\andy\dev\zig\build-release-llvm8\lib\zig\std\special\test_runner.zig:13:25: 0x7ff657908ecb in std.special.main (test.obj)
        if (test_fn.func()) |_| {
                        ^
C:\msys64\home\andy\dev\zig\build-release-llvm8\lib\zig\std\special\start.zig:67:49: 0x7ff657908c8f in ??? (test.obj)
    std.os.windows.kernel32.ExitProcess(callMain());
                                                ^
???:?:?: 0x7ffcea2b4034 in ??? (???)


???:?:?: 0x7ffcebfb3691 in ??? (???)



Tests failed. Use the following command to reproduce the failure:
C:\msys64\home\andy\dev\zig\zig-cache\o\M8vBp-VgTY3kAMkREyfL6R-Yssr4zihm0oJrtl40AsiMjH4brNirFNsCaJI9Bstb\test.exe
The following command exited with error code 2147483651:
c:\msys64\home\andy\dev\zig\build-release-llvm8\bin\zig.exe test C:\msys64\home\andy\dev\zig\test\stage1\behavior.zig --test-name-prefix behavior-windows-x86_64-Debug-bare-multi  --cache-dir C:\msys64\home\andy\dev\zig\zig-cache --name test

So even the behavior tests do not pass on Windows with native CPU features turned on.

@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 Sep 24, 2019
@LemonBoy
Copy link
Contributor

LemonBoy commented Oct 8, 2019

Here's a minimal LLVM IR reproducer:

; Function Attrs: cold nobuiltin noreturn nounwind
define internal void @bar(i1) unnamed_addr #1 {
    unreachable
}

; Function Attrs: nobuiltin nounwind uwtable
define internal fastcc void @test_f128() unnamed_addr #0 {
Entry:
  %x = alloca fp128, align 16
  store fp128 0xL00000000000000003FFF000000000000, fp128* %x, align 16
  %0 = load fp128, fp128* %x, align 16
  %1 = fcmp une fp128 %0, 0xLEB851EB851EB851F400091EB851EB851
  call fastcc void @bar(i1 %1)
  ret void
}

Compile it with the following command:

llc -O0 -mcpu=ivybridge -mtriple x86_64-windows-msvc -filetype asm -o /tmp/foo.s /tmp/foo.ll

You can easily notice how the generated assembly has only a single fp128 constant even though there are two of them in the code above, and the two are definitely not the same value.

The boring news is that the problem is already fixed on LLVM 10.

@andrewrk andrewrk added the upstream An issue with a third party project that Zig uses. label Oct 8, 2019
@andrewrk
Copy link
Member Author

andrewrk commented Oct 8, 2019

The boring news is that the problem is already fixed on LLVM 10.

🎉

Thanks for checking this out. Glad to hear we can do nothing and have this solved in ~6 months.

@andrewrk
Copy link
Member Author

andrewrk commented Oct 8, 2019

If it's obvious what commit fixed it in trunk, we can ask for it to be a 9.0.1 release blocker.

andrewrk added a commit that referenced this issue Jan 22, 2020
See #508. This can be removed when we upgrade to LLVM 10.
andrewrk added a commit that referenced this issue Jan 22, 2020
See #508. These can be re-enabled when we upgrade to LLVM 10.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior os-uefi os-windows standard library This issue involves writing Zig code for the standard library. upstream An issue with a third party project that Zig uses.
Projects
None yet
Development

No branches or pull requests

4 participants