Add {load, store}Unaligned and prefetch wrappers in core.simd #163

baziotis · 2019-08-08T16:37:27Z

Needs a check on the handling of unittests (which are common).

kinke · 2019-08-08T17:21:36Z

src/core/simd.d

+    Returns:
+        Vector
+    */
+    pragma(inline, true);


This statement-form probably doesn't have any effect (should only be used inside a function and then affects that function, but requires semantic analysis of that function), so please get rid of the trailing semicolon.

Yes my bad. But just for info, what did you mean by this?

should only be used inside a function and then affects that function, but requires semantic analysis of that function

What pragma(inline) has to do with the semantic analysis of a function?

Meaning that if the body of the function isn't analyzed, it won't be inlined.

a.d:

void foo() { pragma(inline, true); } // supposedly equivalent, but not in practice: pragma(inline, true) void bar() {}

main.d:

import a; void main() { foo(); bar(); }

ldc2 -output-ll main.d, then viewing main.ll with text editor: foo() call isn't inlined/eliminated. [It is if foo is a template.]

Oh now I see. I guess LDC analyzes functions that have at least one statement. I don't know how a pragma is called formally, but I guess it's not an actual statement.
And so, foo has nothing and so it's body is not analyzed (hence the compiler never sees the pragma). bar is not analyzed either, but the pragma is seen because it's not part of its body.

Is this LDC specific? Also, the reason that you put them in separate files is because in one file, it's one "compilation unit" and so the compiler can reason and just not declare any of them? While I don't know, on a separate it has to declare foo as external symbol.

If it's all in the same file, then the foo-body is fully analyzed (=> codegen into object file), and then the pragma is applied, and all is well. If you're only compiling main.d, then foo isn't codegen'd, the frontend doesn't analyze more than it has to and misses the pragma in the body. And if you compile both modules into one object file (ldc2 -output-ll -singleobj a.d main.d), all is well again. And if you omit the -singleobj and compile the 2 modules into 2 separate object files in a single cmdline, I've just seen that both calls aren't inlined... => bug ;)

Give me a second as I'm trying to comprehend this. To rephrase:

If I try to compile a single file, then a single object file is generated. And the front-end has to analyze all the symbols, as they're in the same file. So everything ok.

Now, if compile main.d only, the compiler has to generate one object file for main.d. That object file calls some functions foo and bar which though will be in some other object file and the only thing that the front-end cares about is the API of these, not the body. So, it misses the body.

The bug is that in 2 object files, only bar should have been inlined? As if just compiling main.d?

kinke · 2019-08-08T17:22:38Z

src/core/simd.d

+            is(V == int4) ||
+            is(V == uint4) ||
+            is(V == long2) ||
+            is(V == ulong2))


Fulfilling the DMD interface doesn't mean we cannot add all vector types and get rid of these 128-bit vectors limitation.

I guess you mean what I added. Although it starts to look a little too much. I think this can be done with some kind of isVector compile-time function.

src/core/simd.d

kinke · 2019-08-08T22:09:26Z

By all vectors, I really meant all ;), not just adding the 256-bit ones (there's also AVX-512 and non-x86 archs...). E.g., even this works:

void foo()
{
    alias V = __vector(float[1023]);
    float[V.length] f = 1;
    import ldc.simd;
    auto v = loadUnaligned!V(f.ptr);
    v += 2;
}

baziotis · 2019-08-08T22:13:59Z

By all vectors, I really meant all ;), not just adding the 256-bit ones.

Hmm.. Ok, let me see how I can do this.

baziotis · 2019-08-08T22:32:00Z

If it's what I uploaded, I'll feel quite dumb haha. I was searching 10 minutes how to pattern match arbitrary types in __vector in the function contract. Like:
if(is(V == __vector(T[len])) where T would be some kind of template parameter and len also, but size_t. -.-

kinke · 2019-08-10T14:45:05Z

The existing unittests (nested test(T)) don't work with 256-bit vectors, they are specialized for 128-bit and need to be generalized if you want to instantiate them with the additional ones.

You can build the druntime test runner like this, in the build dir: ninja druntime-test-runner[-debug][-shared]. Then run the core.simd tests with runtime/druntime-test-runner[-debug][-shared] core.simd.

baziotis · 2019-08-10T16:55:14Z

EDIT: Ignore this message freely.

The existing unittests (nested test(T)) don't work with 256-bit vectors, they are specialized for 128-bit and need to be generalized if you want to instantiate them with the additional ones.

I don't know exactly to handle submodules. druntime is submodule in ldc and so I can't switch to my branch.
But that's not a real problem, I will reapply the commit there and then checkout it. And I built the druntime-test-runner-debug.

But, it seems that there is an error in the old unittests. Or maybe better, something doesn't works that works in DMD

core/simd.d(808): Error: invalid cast from `ubyte` to `void`

That points to this:

druntime/src/core/simd.d

Line 808 in 4995c11

T v;

line.
And it points there even if I remove the new tests.
It seems that the problem is with test!void16() in storeUnaligned tests. If I remove this line, then there are weird linker problems. They're less if I add -shared. But still, there's one:

undefined reference to `_D2rt4util7utility10safeAssertFNbNiNfbMAyaMQemZv'

I guess it's in the assert in the unittests but I don't understand why.

As for the unittests per se, I surely see this problem:

druntime/src/core/simd.d

Line 741 in 4995c11

// Should have enough data to test all 16-byte alignments, and still

whici is it doesn't have enough space for 32 bytes vectors and it doesn't test all 32 alignments. I suppose when you say generalizing, you mean the array size and the alignments.

baziotis · 2019-08-10T19:03:36Z

Sorry I was late on this. I rebuilt LDC and everything ran just fine, meaning, all 4 druntime-test-runner combinations output PASS.

Now, the thing that remains is to generalize the unittests which I guess includes fixing this void16 thing on the storeUnaligned unittest.

kinke · 2019-08-10T19:18:47Z

I suppose when you say generalizing, you mean the array size and the alignments.

Exactly.

I guess includes fixing this void16 thing

Nope, that's a separate issue in the codegenerator and a nice finding - we error out when default-initializing a void-vector, so that needs to be fixed in the compiler.

baziotis · 2019-08-10T19:38:11Z

I suppose when you say generalizing, you mean the array size and the alignments.

Exactly.

Good, but where do we put a stop? Meaning, since you pointed and as I can see here LLVM Vector Type, it doesn't seem that there's a limit on the size.

Edit: Although, one guess is to stop on 512 or 1024 since I think there's no CPU currently supporting more than 512.

Nope, that's a separate issue in the codegenerator and a nice finding - we error out when default-initializing a void-vector, so that needs to be fixed in the compiler.

Ah, great. I'll take a look on the codegen just for fun.

kinke · 2019-08-10T19:47:34Z

I'll take a look on the codegen just for fun.

Compile this with ldc2 -vv -c:

void foo()
{
    alias V = __vector(void[16]);
    V v;
}

=> last lines of output before the error:

VectorExp::toElem() cast(__vector(void[16]))cast(ubyte)0u
* normal (splat) expression
* IntegerExp::toElem: cast(ubyte)0u @ ubyte
* * IntegerExp::toConstElem: cast(ubyte)0u @ ubyte
* * * Building type: ubyte
* * * value = i8 0
* Casting from 'ubyte' to 'void'

You can then grep for VectorExp::toElem() (the string, there's no such function) and try to fix it. :)

baziotis · 2019-08-10T19:56:02Z

Thank you! I see it has the DMD AST structure (which well.. expected since AFAIK it gets an AST from DMD) with toChars() etc. Great!

Edit: Unrelated, but LDC is really -- slow -- to -- build. :P
C++ hits.

baziotis · 2019-08-10T23:43:45Z

It took me 1 hour of running the installed LDC and not the one I was building (hey, it was 12 midnight here).
Another 1 hour that I broke the LDCs I had by generating wrong code (fun times).
And about 2 hours of building LDC and trying to make sense of the codegen.
And here's the PR: ldc-developers/ldc#3130

I feel like it's very hacky. I'll see what the CI says and continue to look at it tomorrow.

kinke · 2019-08-12T09:29:16Z

LDC is really -- slow -- to -- build. C++ hits.

Compared to DMD, yes, but the backend complexity isn't comparable. Rebuilding ninja ldc2 after changing a single .cpp shouldn't take much longer than 10 secs though (most of which is spent for re-linking the ldc2 executable) - that's my experience on Windows. [There are useful CMake options when working on the D files.]

baziotis · 2019-08-12T09:59:58Z

Rebuilding ninja ldc2 after changing a single .cpp shouldn't take much longer than 10 secs

Oh.. it is ninja ldc2. What a satisfactory comment that was. Well, see, I didn't know that. :P
So, every time I was doing ninja. Which takes about 10 minutes, for every change in a .cpp file. :)

that's my experience on Windows. [There are useful CMake options when working on the D files.]

To get LLVM and LDC running on Windows is pretty much the same procedure as in Linux?
I'm especially interested in Visual Studio for the debug experience.
Because for DMD, it's really not at all the same procedure. :)

kinke · 2019-08-12T11:11:16Z

To get LLVM and LDC running on Windows is pretty much the same procedure as in Linux?

More or less, since CMake is cross-platform. The prerequisites, incl. shell setup, are more involved though. See https://wiki.dlang.org/Building_and_hacking_LDC_on_Windows_using_MSVC. Prebuilt LDC-LLVM v7.0.0 (excl. 7.0.0-2) probably works out of the box, so you may be able to skip building LLVM yourself.

baziotis · 2019-08-12T17:22:25Z

Experimental meaning I don't know how good they are. They don't seem that good to me.
For one, you can't use static foreach because of @nogc so there are these ugly mixins.
But also, the whole test function I think needs rethinking because you can't do this:

alias V = __vector(float[1023]);
V v;

because of alignment but can do the code that kinke wrote in a comment above.

baziotis · 2019-08-12T22:19:27Z

My bad that I hadn't run the testsuite. Now that I did, 2 tests fail:
codegen/output_s_affect_codegen.d and codegen/simd_unaligned.d. This is expected but I don't know what should be done. With these wrappers, one can't import core.simd and ldc.simd together and use load/storeUnaligned as there's a conflict.
That seems like a breaking change.
So, do we leave a breaking change and require that the user fully qualifies the intrinsic (i.e. static import) ? Which means that the test cases have to be changed.
Alternatively.. well I can't think of an alternative except for not providing these wrappers.

kinke · 2019-08-12T22:38:16Z

That seems like a breaking change.

Indeed, no good. Maybe we should instead support the upstream signature in ldc.simd and then publicly import the template in core.simd. The upstream implementation could be streamlined with that version too then.

Something like:

private alias ElementType(V) = typeof(V.array[0]);

template loadUnaligned(V)
{
    deprecated("bla please use other version bla")
    V loadUnaligned(const V* p);
    V loadUnaligned(const ElementType!V* p);
}

void foo()
{
    alias float4 = __vector(float[4]);
    const float[4] a;
    static assert(is(typeof(loadUnaligned(cast(const float4*) a.ptr)) == float4));
    static assert(is(typeof(loadUnaligned!float4(a.ptr)) == float4));
}

baziotis · 2019-08-12T23:07:06Z

The upstream implementation could be streamlined with that version too then.

Good indeed! No need for fully qualified name. I'll see what I can do tomorrow.

kinke · 2019-08-14T11:01:56Z

src/ldc/simd.di

+    alias BaseType!V ElementType;
+
+    pragma(inline, true)
+    deprecated("This is the DMD interface, use it only if cross-compiling. Otherwise, please use the LDC interface.")


'Cross-compiling' usually means something completely different, compiling on a host for a different target. 'use it only for DMD compatibility' would be better IMO.

Indeed. I meant to say "if you plan to compile with both DMD and LDC" but too versbose. "DMD compatibility" sells it better.

Let's not add the deprecation message here. If you work on a codebase where deprecations == errors, you force people to immediately start using LDC's interface, while the DMD interface has not been deprecated by Dlang. Adding the deprecation here would mean that LDC would deprecate something that GDC and DMD don't; let's not fork things here :)

Ok, it seems good to have it in a comment though. What do you think?

Adding it to the documentation of the function is fine. dmd -de ... should work as before :)

Isn't -de for features that are officially deprecated? i.e. we don't have control.
https://dlang.org/deprecate.html

If you add deprecated calling the function will no longer compile with -de.

Ah got it :)

kinke · 2019-09-07T21:18:57Z

src/core/simd.d

+            static string generalTests(string T, string size)()
+            {
+                string res;
+                res ~= "test!(__vector("~T~"[8 / "~size~"]))();";


IMO, we can do without these extra tests (the loadUnaligned tests haven't been extended). Sorry if I put you on the wrong track; I said you'd need to generalize the tests if you want to test non-128-bit vectors too. If you do want to keep the tests, then the mixin orgy and passing type and size as strings can be handled more elegantly:

static void generalTests(T)() { static foreach (size; [8, 16, 32, 64]) test!(__vector(T[size / T.sizeof])); } generalTests!void(); generalTests!byte(); ...

Cool indeed, I haven't thought about it. TBH, since we moved the code to ldc/simd, I don't know if there is any point in adding anything to core/simd.

kinke · 2019-09-11T19:26:58Z

I can't push to your branch; could you please resync your local repo with LDC's, git cherry-pick 34f28ceafaff43c32963bab58db421637759251e and then push? I'll squash-merge it then.

baziotis · 2019-09-11T19:59:54Z

I can't push to your branch; could you please resync your local repo with LDC's, git cherry-pick 34f28ceafaff43c32963bab58db421637759251e and then push? I'll squash-merge it then.

I don't know what happened. The command above gives me: fatal: bad object 34f28ceafaff43c32963bab58db421637759251e

Edit: Btw, I did the bad thing of doing the changes in the ldc branch. Maybe that's the problem.

baziotis · 2019-09-11T20:04:19Z

Ok, I fetched the upstream. Now it should be ok.

kinke · 2019-09-11T20:06:36Z

Thx; I meant 'fetch' when saying 'resync', my bad.

baziotis added 2 commits August 8, 2019 19:30

Add {load,store}Unaligned and prefetch wrappers in core.simd

ae1c9d3

Minor doc fix

1aec362

baziotis changed the title ~~Ldc~~ Add {load, store}Unaligned and prefetch wrappers in core.simd Aug 8, 2019

kinke reviewed Aug 8, 2019

View reviewed changes

src/core/simd.d Outdated Show resolved Hide resolved

baziotis added 2 commits August 9, 2019 00:57

prefetch fix and all vector types support

39e24dd

Syntax error and style fix

9f3338e

Support for _all_ vectors

4995c11

Fixes to pass all 4 druntime-test-runner combinations

546e229

Experimental tests for all vectors up to 512 size

f0718d7

Resolve ldc.simd and core.simd conflict

e3bd529

kinke reviewed Aug 14, 2019

View reviewed changes

baziotis added 2 commits August 14, 2019 14:43

Better deprecated message

19d1d3c

Replaced deprecated() message with comment

892388b

kinke reviewed Sep 7, 2019

View reviewed changes

kinke mentioned this pull request Sep 10, 2019

druntime: Add {load,store}Unaligned and prefetch to core.simd ldc-developers/ldc#3160

Merged

baziotis and others added 2 commits September 11, 2019 23:02

Merge remote-tracking branch 'upstream/ldc' into ldc

83bd4a1

Revise

8cb8135

kinke merged commit d995865 into ldc-developers:ldc Sep 11, 2019

kinke mentioned this pull request Sep 11, 2019

Support a few core.simd functions ldc-developers/ldc#3121

Closed

Add {load, store}Unaligned and prefetch wrappers in core.simd #163

Add {load, store}Unaligned and prefetch wrappers in core.simd #163

Conversation

baziotis commented Aug 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kinke Aug 8, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kinke commented Aug 8, 2019

baziotis commented Aug 8, 2019

baziotis commented Aug 8, 2019

kinke commented Aug 10, 2019 • edited Loading

baziotis commented Aug 10, 2019 • edited Loading

baziotis commented Aug 10, 2019

kinke commented Aug 10, 2019

baziotis commented Aug 10, 2019 • edited Loading

kinke commented Aug 10, 2019 • edited Loading

baziotis commented Aug 10, 2019 • edited Loading

baziotis commented Aug 10, 2019

kinke commented Aug 12, 2019 • edited Loading

baziotis commented Aug 12, 2019 • edited Loading

kinke commented Aug 12, 2019 • edited Loading

baziotis commented Aug 12, 2019

baziotis commented Aug 12, 2019

kinke commented Aug 12, 2019 • edited Loading

baziotis commented Aug 12, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kinke commented Sep 11, 2019 • edited Loading

baziotis commented Sep 11, 2019 • edited Loading

baziotis commented Sep 11, 2019

kinke commented Sep 11, 2019

kinke Aug 8, 2019 •

edited

Loading

kinke commented Aug 10, 2019 •

edited

Loading

baziotis commented Aug 10, 2019 •

edited

Loading

baziotis commented Aug 10, 2019 •

edited

Loading

kinke commented Aug 10, 2019 •

edited

Loading

baziotis commented Aug 10, 2019 •

edited

Loading

kinke commented Aug 12, 2019 •

edited

Loading

baziotis commented Aug 12, 2019 •

edited

Loading

kinke commented Aug 12, 2019 •

edited

Loading

kinke commented Aug 12, 2019 •

edited

Loading

kinke commented Sep 11, 2019 •

edited

Loading

baziotis commented Sep 11, 2019 •

edited

Loading