fix Issue 17720 - Wrong code using vector extensions with different types

[ibuclaw]

No description provided.

[dlang-bot]

Thanks for your pull request, @ibuclaw!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	17720	major	Wrong code using vector extensions with different types

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#7065"

[PetarKirov]

Looks good to me.

[ibuclaw]

Hmm, I did not think this one entirely through. Fixed it up with more extensive test, however it looks like DMD's __simd depends on implicitly being able to convert void16 any other vector type of the same size.

Not sure if I should bite my lip and allow this. Maybe @WalterBright might want to chime in as its his intrinsic.

[WalterBright]

It's failing the autotester because of checkwhitespace.

I'm not sure I totally understand your comment, but you can convert an array of ints to an array of void and then to an array of float, and you'll get ints painted to floats. This is allowed behavior.

[WalterBright]

When fixing a bugzilla issue, please put the PR URL in the bugzilla issue, so somebody else doesn't waste time trying to fix it because he's ignorant there is an extant fix already. I already did it for this one.

[ibuclaw]

I'm not sure I totally understand your comment, but you can convert an array of ints to an array of void and then to an array of float, and you'll get ints painted to floats. This is allowed behavior.

__simd accepts and returns a void16. So it expects that any vector is implicitly convertible to and from void.

My original intent here is to make vectors be treated less like an arithmetic type and more like a static/dynamic array. However given the current __simd usage, they need to be a even more relaxed than both.

float[4] sf;
void[16] sv = void;
sv = sf;  // OK
sf = sv;  // Error cannot implictly convert void[16] to float[].

float[] af;
void[] av;
av = af;  // OK
af = av;  // Error cannot implicitly convert void[] to float[].

I was intending to push towards Error cannot implicitly convert void16 to float4.

[ibuclaw]

You can see the error I'm talking about in the autotester now.

[ibuclaw]

@TurkeyMan - maybe you could give a short sentence on whether allowing implicit conversion from void vector to any other vector type seems reasonable.

[TurkeyMan]

How would you argue in favour?
Practically it can be convenient in a lot of typical simd code, but I kinda feel like it's antithetical to D in general. It's certainly not @safe ;)
I reckon it'd be better to put a bunch of overloads/wrappers in core.simd to match return types to the argument types like most C++ simd headers do?

[ibuclaw]

Added overrides in dlang/druntime#1900 - however using one of them exposes an ICE, others seem fine.

[ibuclaw]

Blocked by druntime and dmd bug for the moment (this patch is already in gdc, so I have a vested interest to see that someone fixes it).

[ibuclaw]

All checks pass, I'll leave it up to the RM @thewilsonator to decide if any action is required for forcing casts when using __simd().

[ibuclaw]

Added changelog entry.

[ibuclaw]

Github UI doesn't seem to be updating to reveal the new commit...

[ghost]

Why dont you choose to start a deprecation ?

[ibuclaw]

Why dont you choose to start a deprecation ?

GDC has been running with this for as long as the bug report has been open.

Actually I'd like to make this even more restricted. Implicit casting between shared and non-shared is still allowed, but shouldn't be. However that will really begin to break code, so will need a deprecation.

This just makes an error code that is broken anyway.

[ghost]

So the problem is just that the PR has been forgotten but it's good to merge, right ?

[ibuclaw]

So the problem is just that the PR has been forgotten but it's good to merge, right ?

Pretty much. The only blocking part was the dmd intrinsic __simd, which has the signature void16(void16, void16). This is a dumb intrinsic, and new overrides should be added to deal with both allowing AVX instructions and returning a result other than void (eg: float4)