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

[AArch64][Win] Work around an MSVC arm64 compiler bug #67865

Merged
merged 1 commit into from
Oct 2, 2023

Conversation

hjyamauchi
Copy link
Contributor

The MSVC compiler 19.37 for ARM64 from Visual Studio 17.7.4 has an optimization bug that causes an incorrect behavior with isAdvSIMDModImmType10() and causes the test
test/CodeGen/AArch64/arm64-build-vector.ll to fail. Work around by using a slightly different variation.

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 29, 2023

@llvm/pr-subscribers-backend-aarch64

Changes

The MSVC compiler 19.37 for ARM64 from Visual Studio 17.7.4 has an optimization bug that causes an incorrect behavior with isAdvSIMDModImmType10() and causes the test
test/CodeGen/AArch64/arm64-build-vector.ll to fail. Work around by using a slightly different variation.


Full diff: https://github.com/llvm/llvm-project/pull/67865.diff

1 Files Affected:

  • (modified) llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h (+24)
diff --git a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h
index 11c314dc88def7e..e6c52c9ce60d367 100644
--- a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h
+++ b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h
@@ -591,6 +591,29 @@ static inline uint64_t decodeAdvSIMDModImmType9(uint8_t Imm) {
 // aaaaaaaa bbbbbbbb cccccccc dddddddd eeeeeeee ffffffff gggggggg hhhhhhhh
 // cmode: 1110, op: 1
 static inline bool isAdvSIMDModImmType10(uint64_t Imm) {
+#if defined(_MSC_VER) && _MSC_VER == 1937 && !defined(__clang__) && defined(_M_ARM64)
+// The MSVC compiler 19.37 for ARM64 has an optimization bug that
+// causes an incorrect behavior with the orignal version. Work around
+// by using a slightly different variation.
+  constexpr uint64_t Mask = 0xFFULL;
+  uint64_t ByteA = (Imm >> 56) & Mask;
+  uint64_t ByteB = (Imm >> 48) & Mask;
+  uint64_t ByteC = (Imm >> 40) & Mask;
+  uint64_t ByteD = (Imm >> 32) & Mask;
+  uint64_t ByteE = (Imm >> 24) & Mask;
+  uint64_t ByteF = (Imm >> 16) & Mask;
+  uint64_t ByteG = (Imm >> 8) & Mask;
+  uint64_t ByteH = Imm & Mask;
+
+  return (ByteA == 0ULL || ByteA == Mask) &&
+         (ByteB == 0ULL || ByteB == Mask) &&
+         (ByteC == 0ULL || ByteC == Mask) &&
+         (ByteD == 0ULL || ByteD == Mask) &&
+         (ByteE == 0ULL || ByteE == Mask) &&
+         (ByteF == 0ULL || ByteF == Mask) &&
+         (ByteG == 0ULL || ByteG == Mask) &&
+         (ByteH == 0ULL || ByteH == Mask);
+#else
   uint64_t ByteA = Imm & 0xff00000000000000ULL;
   uint64_t ByteB = Imm & 0x00ff000000000000ULL;
   uint64_t ByteC = Imm & 0x0000ff0000000000ULL;
@@ -608,6 +631,7 @@ static inline bool isAdvSIMDModImmType10(uint64_t Imm) {
          (ByteF == 0ULL || ByteF == 0x0000000000ff0000ULL) &&
          (ByteG == 0ULL || ByteG == 0x000000000000ff00ULL) &&
          (ByteH == 0ULL || ByteH == 0x00000000000000ffULL);
+#endif
 }
 
 static inline uint8_t encodeAdvSIMDModImmType10(uint64_t Imm) {

@hjyamauchi
Copy link
Contributor Author

Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

That is atrocious. Could you please add the connect/DevComm link for the report?

@hjyamauchi
Copy link
Contributor Author

That is atrocious. Could you please add the connect/DevComm link for the report?

Done.

@hjyamauchi
Copy link
Contributor Author

Rebased

@github-actions
Copy link

github-actions bot commented Sep 29, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

The MSVC compiler 19.37 for ARM64 from Visual Studio 17.7.4 has an
optimization bug that causes an incorrect behavior with
isAdvSIMDModImmType10() and causes the test
test/CodeGen/AArch64/arm64-build-vector.ll to fail. Work around by
using a slightly different variation.

Bug: https://developercommunity.visualstudio.com/t/C-ARM64-compiler-optimization-bug/10481261
@hjyamauchi
Copy link
Contributor Author

Fixed formatting

@davemgreen
Copy link
Collaborator

This is a bit of a shame but the code looks OK.

@hjyamauchi hjyamauchi merged commit f776e0b into llvm:main Oct 2, 2023
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants