Add support for non aligned SIMD for vec4 #1162
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
laurentcau
force-pushed
the
master
branch
2 times, most recently
from
942c5e2
to
ae54b47
Compare
It is used as default configuration for Visual Studio 64 bits compilation (needs Language Extension). code changes: - add new qualifiers: unaligned_simd_highp unaligned_simd_mediump unaligned_simd_lowp - add use_simd and replace is_aligned (code for ARM NEON is added but not tested)
tycho
reviewed
Nov 19, 2023
| }; | ||
|
|
||
|
|
||
| return ; |
Comment on lines
+313
to
+314
| type(glm_i32vec4 v) { vst1q_u32(data, v); } | ||
| operator glm_i32vec4() const { return vld1q_u32(data); } |
There was a problem hiding this comment.
These should be vst1q_s32 and vld1q_s32, since the vector is signed.
Comment on lines
+331
to
+332
| type(glm_i32vec4 v) { vst1q_u32(data, v); } | ||
| operator glm_i32vec4() const { return vld1q_u32(data); } |
There was a problem hiding this comment.
glm_i32vec4 should be replaced with glm_u32vec4 here.
|
Hi,
Feal free to fix the mistake. I have no way to test ARM version and glm validation process only support Windows Visual Studio build.
If you know an easy way to test this on PC, share it, please.
I have another big Pull Request that will surely break ARM version too. (add simd (aligned and unaligned) operation for vec3 and dvec3 by laurentcau · Pull Request #1164 · g-truc/glm · GitHub<#1164>)
Regards
Laurent
From: Steven Noonan ***@***.***>
Sent: Sunday, November 19, 2023 6:14 PM
To: g-truc/glm ***@***.***>
Cc: Laurent Caumont ***@***.***>; Author ***@***.***>
Subject: Re: [g-truc/glm] Add support for non aligned SIMD for vec4 (PR #1162)
WARNING: This email originated from outside of the organization. DO NOT click links, open attachments, or respond unless you recognize the sender and know the content is safe.
________________________________
@tycho commented on this pull request.
Some bugs in this contribution that I just ran into when trying to build targeting NEON.
________________________________
In glm/detail/qualifier.hpp<https://urldefense.com/v3/__https:/github.com/g-truc/glm/pull/1162*discussion_r1398459398__;Iw!!F1Q1IbZmrAg!BxtWP7nnSsrdB8JE1cF1dj3Ed4_0cN5wlaHAYOAyXAoQSzXmohF9-yT5ZlHiZhu-lpIXabtlD57-ymQdxKhA1H8zfqEzyQ$>:
@@ -173,17 +283,56 @@ namespace detail
typedef glm_f32vec4 type;
};
+ template<>
+ struct storage<4, float, false, true>
+ {
+ typedef struct type {
+ float data[4];
+ GLM_DEFAULTED_DEFAULT_CTOR_QUALIFIER GLM_CONSTEXPR type() GLM_DEFAULT;
+ inline type(glm_f32vec4 v) { vst1q_f32(reinterpret_cast<float*>(data), v); }
+ inline operator glm_f32vec4() const { return vld1q_f32(reinterpret_cast<const float*>(data)); }
+ } type;
+ };
+
+
+ return ;
This line shouldn't be here.
________________________________
In glm/detail/qualifier.hpp<https://urldefense.com/v3/__https:/github.com/g-truc/glm/pull/1162*discussion_r1398459627__;Iw!!F1Q1IbZmrAg!BxtWP7nnSsrdB8JE1cF1dj3Ed4_0cN5wlaHAYOAyXAoQSzXmohF9-yT5ZlHiZhu-lpIXabtlD57-ymQdxKhA1H90lCHkfQ$>:
+ type(glm_i32vec4 v) { vst1q_u32(data, v); }
+ operator glm_i32vec4() const { return vld1q_u32(data); }
These should be vst1q_s32 and vld1q_s32, since the vector is signed.
________________________________
In glm/detail/qualifier.hpp<https://urldefense.com/v3/__https:/github.com/g-truc/glm/pull/1162*discussion_r1398459699__;Iw!!F1Q1IbZmrAg!BxtWP7nnSsrdB8JE1cF1dj3Ed4_0cN5wlaHAYOAyXAoQSzXmohF9-yT5ZlHiZhu-lpIXabtlD57-ymQdxKhA1H_H1-jgfg$>:
+ type(glm_i32vec4 v) { vst1q_u32(data, v); }
+ operator glm_i32vec4() const { return vld1q_u32(data); }
glm_i32vec4 should be replaced with glm_u32vec4 here.
—
Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https:/github.com/g-truc/glm/pull/1162*pullrequestreview-1738686385__;Iw!!F1Q1IbZmrAg!BxtWP7nnSsrdB8JE1cF1dj3Ed4_0cN5wlaHAYOAyXAoQSzXmohF9-yT5ZlHiZhu-lpIXabtlD57-ymQdxKhA1H-irOUM2Q$>, or unsubscribe<https://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/AJWNZ2SH6LNOTOHV2GUZPN3YFI44FAVCNFSM6AAAAAA7BG6FQWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMZYGY4DMMZYGU__;!!F1Q1IbZmrAg!BxtWP7nnSsrdB8JE1cF1dj3Ed4_0cN5wlaHAYOAyXAoQSzXmohF9-yT5ZlHiZhu-lpIXabtlD57-ymQdxKhA1H9DmqKNjQ$>.
You are receiving this because you authored the thread.Message ID: ***@***.******@***.***>>
…________________________________
This email, including any attachments, may contain confidential and/or proprietary information intended only for the use of the recipient. If you are not the intended recipient, any distribution, copying, or use of this email or its attachments is prohibited. If you received this email in error, please reply to the sender immediately and delete this message and any copies.
Bentley Systems has taken all reasonable steps to ensure that this communication is free from viruses, data corruption, and unauthorized alteration. Bentley Systems does not accept liability for any damages that may be incurred as a result of this or any communication by email
[https://cdn2.webdamdb.com/220th_sm_EdpYPzNrja101cmW.jpg?1691764309]
|
Well, I built with clang-cl targeting ARM64 on Visual Studio to find these issues. If you install clang-cl and the other ARM64 bits via the Visual Studio Installer, you should be able to test that. You can configure with CMake using either |
|
Thank you for the tip.
I was able to setup a build. It compiles fine BUT it doesn’t use the NEON path.
125>D:\repo\ext\glm\glm/gtc/../ext/../detail/setup.hpp(948,11): warning : "GLM: version 0.9.9.9" [-W#pragma-messages]
125>D:\repo\ext\glm\glm/gtc/../ext/../detail/setup.hpp(960,11): warning : GLM: C++ 14 with extensions [-W#pragma-messages]
125>D:\repo\ext\glm\glm/gtc/../ext/../detail/setup.hpp(991,11): warning : GLM: Clang compiler detected [-W#pragma-messages]
125>D:\repo\ext\glm\glm/gtc/../ext/../detail/setup.hpp(1061,11): warning : GLM: Unknown build target [-W#pragma-messages]
125>D:\repo\ext\glm\glm/gtc/../ext/../detail/setup.hpp(1074,11): warning : GLM: Windows platform detected [-W#pragma-messages]
125>D:\repo\ext\glm\glm/gtc/../ext/../detail/setup.hpp(1096,11): warning : GLM: GLM_FORCE_SWIZZLE is defined, swizzling operators enabled. [-W#pragma-messages]
125>D:\repo\ext\glm\glm/gtc/../ext/../detail/setup.hpp(1107,11): warning : GLM: GLM_FORCE_SIZE_T_LENGTH is undefined. .length() returns a glm::length_t, a typedef of int following GLSL. [-W#pragma-messages]
125>D:\repo\ext\glm\glm/gtc/../ext/../detail/setup.hpp(1113,11): warning : GLM: GLM_FORCE_UNRESTRICTED_GENTYPE is undefined. Follows strictly GLSL on valid function genTypes. [-W#pragma-messages]
125>D:\repo\ext\glm\glm/gtc/../ext/../detail/setup.hpp(1119,11): warning : GLM: GLM_FORCE_SILENT_WARNINGS is undefined. Shows C++ warnings from using C++ language extensions. [-W#pragma-messages]
125>D:\repo\ext\glm\glm/gtc/../ext/../detail/setup.hpp(1146,11): warning : GLM: GLM_FORCE_DEPTH_ZERO_TO_ONE is undefined. Using negative one to one depth clip space. [-W#pragma-messages]
125>D:\repo\ext\glm\glm/gtc/../ext/../detail/setup.hpp(1152,11): warning : GLM: GLM_FORCE_LEFT_HANDED is undefined. Using right handed coordinate system. [-W#pragma-messages]
Is there any option I should add to cmake to use it ?
From: Steven Noonan ***@***.***>
Sent: Wednesday, November 22, 2023 11:03 AM
To: g-truc/glm ***@***.***>
Cc: Laurent Caumont ***@***.***>; Author ***@***.***>
Subject: Re: [g-truc/glm] Add support for non aligned SIMD for vec4 (PR #1162)
WARNING: This email originated from outside of the organization. DO NOT click links, open attachments, or respond unless you recognize the sender and know the content is safe.
…________________________________
Hi, Feal free to fix the mistake. I have no way to test ARM version and glm validation process only support Windows Visual Studio build. If you know an easy way to test this on PC, share it, please.
Well, I built with clang-cl targeting ARM64 on Visual Studio to find these issues. If you install clang-cl and the other ARM64 bits via the Visual Studio Installer, you should be able to test that. You can configure with CMake using either -G "Visual Studio 16 2019" -A ARM64 -T ClangCL or -G "Visual Studio 16 2019" -A ARM64 -T LLVM_v142, depending on which toolset is installed.
—
Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https:/github.com/g-truc/glm/pull/1162*issuecomment-1822457638__;Iw!!F1Q1IbZmrAg!GP3sO0HybWPPT4OmVylI4fviUYEXP4MksV8k6AK5rUNa4fWYRdBolLomXFSMVxddMJA_YbTDLNgoOj-4VPAQmREpxqnsoA$>, or unsubscribe<https://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/AJWNZ2XZ2HCN3HA4A7MTEKLYFXEVXAVCNFSM6AAAAAA7BG6FQWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMRSGQ2TONRTHA__;!!F1Q1IbZmrAg!GP3sO0HybWPPT4OmVylI4fviUYEXP4MksV8k6AK5rUNa4fWYRdBolLomXFSMVxddMJA_YbTDLNgoOj-4VPAQmRGW5z2E9Q$>.
You are receiving this because you authored the thread.Message ID: ***@***.******@***.***>>
________________________________
This email, including any attachments, may contain confidential and/or proprietary information intended only for the use of the recipient. If you are not the intended recipient, any distribution, copying, or use of this email or its attachments is prohibited. If you received this email in error, please reply to the sender immediately and delete this message and any copies.
Bentley Systems has taken all reasonable steps to ensure that this communication is free from viruses, data corruption, and unauthorized alteration. Bentley Systems does not accept liability for any damages that may be incurred as a result of this or any communication by email
[https://cdn2.webdamdb.com/220th_sm_EdpYPzNrja101cmW.jpg?1691764309]
|
|
Oh yeah, my branch has patches to make it detect ARM Windows ( |
|
Hi Laurent, I revert the commit because it cause too much issues. I am working on fixing the repository C.I. with #1167 which I hope will help avoiding such problem in the future... Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It is used as default configuration for Visual Studio 64 bits compilation (needs Language Extension).
code changes:
(code for ARM NEON is added but not tested)