Skip to content

[SPIR-V] Fix type compatibility in memory order comparisons #123676

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

Conversation

michalpaszkowski
Copy link
Member

@michalpaszkowski michalpaszkowski commented Jan 20, 2025

Fixed a type mismatch issue in the comparison of std::memory_order with integers.

This fixes an issue reported by clang-debian-cpp20 buildbot for #123654

@llvmbot
Copy link
Member

llvmbot commented Jan 20, 2025

@llvm/pr-subscribers-backend-spir-v

Author: Michal Paszkowski (michalpaszkowski)

Changes

Fixed a type mismatch issue in the comparison of std::memory_order with unsigned.

This fixes an issue reported by clang-debian-cpp20 buildbot for #123654


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

1 Files Affected:

  • (modified) llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp (+6-6)
diff --git a/llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp b/llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp
index 784bbe8e662c20..d3976f645e6832 100644
--- a/llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp
@@ -606,13 +606,13 @@ static Register buildMemSemanticsReg(Register SemanticsRegister,
                                      SPIRVGlobalRegistry *GR) {
   if (SemanticsRegister.isValid()) {
     MachineRegisterInfo *MRI = MIRBuilder.getMRI();
-    std::memory_order Order =
-        static_cast<std::memory_order>(getIConstVal(SemanticsRegister, MRI));
-    Semantics =
-        getSPIRVMemSemantics(Order) |
+    int MemoryOrderValue = getIConstVal(SemanticsRegister, MRI);
+    std::memory_order Order = static_cast<std::memory_order>(MemoryOrderValue);
+    unsigned OrderSemantics = getSPIRVMemSemantics(Order);
+    unsigned StorageClassSemantics =
         getMemSemanticsForStorageClass(GR->getPointerStorageClass(PtrRegister));
-
-    if (Order == Semantics) {
+    Semantics = OrderSemantics | StorageClassSemantics;
+    if (OrderSemantics == Semantics) {
       MRI->setRegClass(SemanticsRegister, &SPIRV::iIDRegClass);
       return SemanticsRegister;
     }

@michalpaszkowski michalpaszkowski force-pushed the fix_SPIRVBuiltins_invalid_operand_types branch from 594d447 to 8110eff Compare January 21, 2025 00:09
@michalpaszkowski michalpaszkowski changed the title [SPIR-V] Refactor buildMemSemanticsReg to ensure type compatibility [SPIR-V] Fix buildMemSemanticsReg to ensure type compatibility Jan 21, 2025
Fixed a type mismatch issue in the comparison of std::memory_order with
integers.

This fixes an issue reported by clang-debian-cpp20 buildbot for
llvm#123654
@michalpaszkowski michalpaszkowski force-pushed the fix_SPIRVBuiltins_invalid_operand_types branch from 8110eff to 8d327e6 Compare January 21, 2025 00:19
@michalpaszkowski michalpaszkowski changed the title [SPIR-V] Fix buildMemSemanticsReg to ensure type compatibility [SPIR-V] Fix type compatibility in memory order comparisons Jan 21, 2025
@michalpaszkowski michalpaszkowski merged commit b45072d into llvm:main Jan 21, 2025
9 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.

3 participants