Skip to content

Merge 2.073.2 front-end#2006

Merged
kinke merged 23 commits intomasterfrom
merge-2.073
Mar 18, 2017
Merged

Merge 2.073.2 front-end#2006
kinke merged 23 commits intomasterfrom
merge-2.073

Conversation

@kinke
Copy link
Member

@kinke kinke commented Feb 18, 2017

No description provided.

@kinke
Copy link
Member Author

kinke commented Feb 18, 2017

The compiler itself can be built on Win64, but compiling the libs fails for each (?) module due to 'recursive template expansion'.

@kinke
Copy link
Member Author

kinke commented Feb 18, 2017

Alright, compiler and libs build successfully now (incl. self-compile). The single remaining unittest compilation error is due to some sort of forward-referencing or other symbol resolution failure for std.regex. The first dmd-testsuite results are promising.
mars.d has changed quite a bit, primarily due to the new DMD -mcpu switch, with obviously incompatible values compared to LDC; a mapping to LLVM CPU and/or CPU features will probably be required for LDMD.

Ignore `-mcpu=baseline`, forward `-mcpu=native` to LDC and translate
`-mcpu=avx` to `-mattr=+avx`. Show LLVM's (extensive) help for `-mcpu=?`
(which is sadly actually printed 3 consecutive times).

if (arch == llvm::Triple::x86 || arch == llvm::Triple::x86_64) {
if (traitsTargetHasFeature("sse2")) {
VersionCondition::addPredefinedGlobalIdent("D_SIMD");
Copy link
Member

@dnadlinger dnadlinger Feb 19, 2017

Choose a reason for hiding this comment

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

D_SIMD refers to the DMD-style __simd opcode interface. (The docs are vague on this. Edit again: https://dlang.org/spec/version.html is clearer.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah okay, and D_AVX isn't documented at all, but I guess it's just an extension for additional opcodes.

dnadlinger
dnadlinger previously approved these changes Feb 19, 2017
@dnadlinger
Copy link
Member

https://dlang.org/changelog/2.073.0.html says "Add D_AVX predefined version when -mcpu=avx is used", so D_AVX might have been correct.

@dnadlinger dnadlinger dismissed their stale review February 19, 2017 02:07

(Misunderstood UI.)

@kinke
Copy link
Member Author

kinke commented Feb 19, 2017

This concludes propagating the mars.d changes to LDC/LDMD. On Win64, there are 5 remaining failures wrt. dmd-testsuite-debug alone, 3 of which seem invalid.

@kinke
Copy link
Member Author

kinke commented Feb 19, 2017

Update:

  • unittests: std.regex.internal.shiftor fails to compile its unittests ('template not defined'), all others compile & work (at least on Win64).
  • lit-tests: work.
  • dmd-testsuite: works, except for:
    • runnable/b16278.d: missing symbol on Win64/OSX (debug only), links on Linux (?)
    • runnable/test15862.d: crash ('illegal instruction')

@kinke
Copy link
Member Author

kinke commented Feb 22, 2017

runnable/test15862.d shows that LDC still lacks propagating immutable constants to LL constants. The 4 failing asserts check that repeated calls to pure functions returning indirections to immutable stuff return pointers to the same LL constant.

@dnadlinger
Copy link
Member

dnadlinger commented Feb 22, 2017

runnable/test15862.d … immutable constants to LL constants

DMD passes this not due to emitting stuff as constant, but because it does CSE on strongly pure functions. Actually, the assembly for e.g. the on() part is … rather funny:

   100000db4:   e8 b7 33 02 00          call   100024170          // allocate object
   100000db9:   48 89 45 f8             mov    QWORD PTR [rbp-0x8],rax
   100000dbd:   48 8b 4d f8             mov    rcx,QWORD PTR [rbp-0x8]
   100000dc1:   48 3b c8                cmp    rcx,rax

In other words, it copies the pointer to the allocated object to the stack, loads it into another register and then compares the two. Solid.

@dnadlinger
Copy link
Member

(Just disabling the test until we implement optimizations on strongly pure functions – if ever – would be fine.)

@kinke
Copy link
Member Author

kinke commented Feb 22, 2017

So DMD caches the first result for all later detected identical invokations of strongly pure functions inside a function? Apparently only when optimizing. Shouldn't/couldn't this be done in the front-end already? At least optionally, in case the compile time/memory footprint impact could become noticeable.

@dnadlinger
Copy link
Member

Shouldn't/couldn't this be done in the front-end already?

It could certainly be done as a pass over the AST, yes (whether you call that front-end or not).

Instead of letting our old copy silently go more and more out-of-sync.
In this case, we missed the upstream fix wrt. skipping the declaration of
ClassInfos for speculative class types.

There are 2 noteworthy functional changes for function
`getOrCreateTypeInfoDeclaration()`:

1) The old version always overwrote `torig->vtinfo` with `t->vtinfo` when
   declaring a new TypeInfo, whereas upstream's `genTypeInfo()` only sets
   it if it was null before.

2) The old version called `semanticTypeInfo()` during a semantic pass,
   upstream doesn't.

The LDC-specific exception for class types in `builtinTypeInfo()` is still
required.

Fixes dmd-testsuite's runnable/b16278.d.
@kinke
Copy link
Member Author

kinke commented Mar 5, 2017

Apparently only remaining issue: std.regex.internal.shiftor unittest compile error.

@kinke
Copy link
Member Author

kinke commented Mar 5, 2017

Wow, that std.regex thing seems really ugly.

alias std.regex.internal.kickstart.Kickstart = std.regex.internal.kickstart.ShiftOr not being available in module std.regex.internal.shiftor may be okay (?), as it's only imported locally in a struct in imported std.regex.internal.ir.

When trying to work around this via explicit import:

diff --git a/std/regex/internal/shiftor.d b/std/regex/internal/shiftor.d
index 0a549f4..f3aca3a 100644
--- a/std/regex/internal/shiftor.d
+++ b/std/regex/internal/shiftor.d
@@ -6,7 +6,7 @@ module std.regex.internal.shiftor;

 package(std.regex):

-import std.regex.internal.ir;
+import std.regex.internal.ir, std.regex.internal.kickstart : Kickstart;
 import std.range.primitives, std.utf;

 //utility for shiftOr, returns a minimum number of bytes to test in a Char

it then fails with

C:\LDC\ldc\runtime\phobos\std\regex\internal\shiftor.d(29): Error: class std.regex.internal.shiftor.ShiftOr!char.ShiftOr base type must be class or interface, not Kickstart!char

which makes perfect sense, as the (very similar) base std.regex.internal.kickstart.ShiftOr!char is a struct, not a class!!

Pinging @JackStouffer due to dlang/phobos@5a2491a. Apparently there was a std.regex.internal.ir.Kickstart interface before the revert.

@kinke
Copy link
Member Author

kinke commented Mar 5, 2017

Upstream PR: dlang/phobos#5244

Awaiting some green now. :)

@kinke
Copy link
Member Author

kinke commented Mar 5, 2017

One more semantic/target_traits.d lit test failure (LDC crash) on Win32 (possibly x86 in general; the x86 AppVeyor job is the only one running the lit-tests in 32-bit, there's no multilib config for it yet) and accordingly no results for the Win32 unittests yet; green on all other CI systems (minus spurious OSX core.thread failure).

A ctor (I think I added it for convenience when merging 2.073) means that
it's no POD type for C++ anymore. Non-POD structs are expected to be
returned via sret etc.

Fixes lit-test semantic/target_traits.d for x86 hosts.
@kinke kinke changed the title Merge 2.073.1 front-end Merge 2.073.2 front-end Mar 11, 2017
@kinke
Copy link
Member Author

kinke commented Mar 11, 2017

Upgraded to 2.073.2 (very few fixes only). The first of the 2 front-end fixes originated from this PR (06e88fe), the other one was already cherry-picked into master (b159de0).

@dushibaiyu
Copy link

will it marge 073.2 into ldc 1.2.0 release?

@dnadlinger
Copy link
Member

@dushibaiyu: No, 2.073.2 will go into 1.3.x.

@dnadlinger
Copy link
Member

@kinke: I can finally reproduce the spurious OS X core.thread issue on my local machine!

druntime-test-runner(43837,0x7fffb68223c0) malloc: *** error for object 0x1: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
Abort trap: 6

Investigating…

@dnadlinger
Copy link
Member

Ah, the issue seems to occur during garbage collection of stale Thread objects:

* thread #1: tid = 0x0000, 0x00007fffadb03dd6 libsystem_kernel.dylib`__pthread_kill + 10, stop reason = signal SIGSTOP
  * frame #0: 0x00007fffadb03dd6 libsystem_kernel.dylib`__pthread_kill + 10
    frame #1: 0x00007fffadbef787 libsystem_pthread.dylib`pthread_kill + 90
    frame #2: 0x00007fffada69420 libsystem_c.dylib`abort + 129
    frame #3: 0x00007fffadb5903f libsystem_malloc.dylib`free + 530
    frame #4: 0x000000010f87c537 druntime-test-runner`_D2rt4util9container6common8xreallocFNbNiPvmZPv + 71
    frame #5: 0x000000010f874869 druntime-test-runner`_D2rt19sections_elf_shared13finiTLSRangesFPS2rt4util9container5array13__T5ArrayTAvZ5ArrayZv + 73
    frame #6: 0x000000010f875d0c druntime-test-runner`_D2rt5tlsgc7destroyFPvZv + 12
    frame #7: 0x000000010f7dfccd druntime-test-runner`_D4core6thread6Thread6__dtorMFZv + 45
    frame #8: 0x000000010f86800e druntime-test-runner`rt_finalize2 + 94
    frame #9: 0x000000010f81a56a druntime-test-runner`_D2gc4impl12conservative2gc3Gcx5sweepMFNbZm + 314
    frame #10: 0x000000010f817165 druntime-test-runner`_D2gc4impl12conservative2gc3Gcx11fullcollectMFNbbZm + 533
    frame #11: 0x000000010f81734d druntime-test-runner`_D2gc4impl12conservative2gc14ConservativeGC150__T9runLockedS100_D2gc4impl12conservative2gc14ConservativeGC11fullCollectMFNbZ2goFNbPS2gc4impl12conservative2gc3GcxZmTPS2gc4impl12conservative2gc3GcxZ9runLockedMFNbKPS2gc4impl12conservative2gc3GcxZm + 45
    frame #12: 0x000000010f7e77da druntime-test-runner`_D4core6thread18__unittestL5742_34FZv + 122
    frame #13: 0x000000010f7ea07a druntime-test-runner`_D4core6thread10__unittestZ + 410
    frame #14: 0x000000010f7ba2c0 druntime-test-runner`_D11test_runner6doTestFPS6object10ModuleInfoKbZv + 64
    frame #15: 0x000000010f7ba1d0 druntime-test-runner`_D11test_runner11testModulesFZb + 176
    frame #16: 0x000000010f7d9d75 druntime-test-runner`runModuleUnitTests + 101
    frame #17: 0x000000010f863e46 druntime-test-runner`_d_run_main + 470
    frame #18: 0x00007fffad9d5255 libdyld.dylib`start + 1
    frame #19: 0x00007fffad9d5255 libdyld.dylib`start + 1

Seems like it might be related to dlang/druntime#1655, which applied to the Shared path.

@dushibaiyu
Copy link

@klickverbot Thanks.

@dnadlinger
Copy link
Member

Seems like it might be related to dlang/druntime#1655

Yep, this is indeed the same issue. We should probably just move _tlsRanges to be on the C heap, with only the pointer being stored in TLS.

@dnadlinger
Copy link
Member

dnadlinger commented Mar 18, 2017

@kinke: I pushed a patch which should fix the OS X core.thread crashes (if they were indeed the same as I got locally).

Happy to merge the branch to master after green?

@kinke
Copy link
Member Author

kinke commented Mar 18, 2017

Great that the OSX issue seems to be understood now (although it'll still happen for the shared libs as I understand).
We'll need it for LDC 1.2 too though, right?
Oh and merging is fine by me (and consequent cherry-picking only for branch release-1.2.[0x]). We'll be able to merge a bunch of other PRs deferred to 1.3 then too (#2016, #1960, #2030).

@dnadlinger
Copy link
Member

although it'll still happen for the shared libs as I understand

This specific issue should have been fixed already for shared libraries.

@kinke
Copy link
Member Author

kinke commented Mar 18, 2017

I see, since 2.072. I don't remember whether it was only the static-libs job spuriously failing since 2.072, but we'll find out.

@kinke
Copy link
Member Author

kinke commented Mar 18, 2017

Looking good. I'll cherry-pick the fix onto master before merging 2.073.2 so that 1.2 doesn't (yet) need its own ldc-release-1.2.x druntime branch.

@dnadlinger
Copy link
Member

Okay I'll let you do the merge, then.

Conflicts:
	runtime/druntime
Conflicts:
	runtime/druntime
@kinke kinke merged commit 5c5831a into master Mar 18, 2017
@kinke kinke deleted the merge-2.073 branch March 18, 2017 22:02
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