[AMDGPU] Avoid using tuple where pair does suffice

[maurossi]

Summary

Fixes the following building errors, happening with official Android prebuilt clang 14 shipped with Android 13:

external/llvm-project/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:5491:13: error: no viable constructor or deduction g
uide for deduction of template arguments of 'tuple'

? std::tuple(HSAMD::V3::AssemblerDirectiveBegin,
  ^

...
external/llvm-project/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:5493:13: error: no viable constructor or deduction g
uide for deduction of template arguments of 'tuple'

: std::tuple(HSAMD::AssemblerDirectiveBegin,
  ^

Fixes: 6443c0e ("[AMDGPU] Stop using make_pair and make_tuple. NFC.")

[maurossi]

Hello,

I have two questions

Since the issue happens also in release/16.x branch will D142839 be backported to release/16.x branch ?

Why has "x64 debian" build failed?

[foad]

Why has "x64 debian" build failed?

Looks like the part that failed is checking that the lines you changed are formatted properly according to "clang-format". You can reproduce this locally by running .../llvm-project/clang/tools/clang-format/git-clang-format @^ and seeing if it updates any of the files you changed.

[maurossi]

Hi Jay,
here is the output of 'git-clang-format --diff'

utente@utente-3TB:~/r-x86_kernel/external/llvm-project$ ./clang/tools/clang-format/git-clang-format --diff
diff --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index c879b4914ee7..7ee1672049b3 100644
--- a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -5488,9 +5488,9 @@ bool AMDGPUAsmParser::ParseDirectiveHSAMetadata() {
   std::tie(AssemblerDirectiveBegin, AssemblerDirectiveEnd) =
       isHsaAbiVersion3AndAbove(&getSTI())
           ? std::pair(HSAMD::V3::AssemblerDirectiveBegin,
-                       HSAMD::V3::AssemblerDirectiveEnd)
+                      HSAMD::V3::AssemblerDirectiveEnd)
           : std::pair(HSAMD::AssemblerDirectiveBegin,
-                       HSAMD::AssemblerDirectiveEnd);
+                      HSAMD::AssemblerDirectiveEnd);
 
   if (getSTI().getTargetTriple().getOS() != Triple::AMDHSA) {
     return Error(getLoc(),

[maurossi]

Diff updated for conformance with git-clang-format

[foad]

Patch LGTM, thanks. But I don't understand why the Android toolchain was complaining. Does it claim to support C++17?

[xiaoqili]

Same issue happened on Android prebuilt clang 8 in NDK20. Seem it's having a stricter rule for deduction the template arguments.
Root cause should be the different definition of AssemblerDirectiveBegin/AssemblerDirectiveEnd.
It's defined as "const char * ......"in std::tie which is the left value.
But the right value which in HSAMD::V3 and HSAMD:: it's defined as "constexpr char ......[]".
Tried to make it defined as "const char * ......" in HSAMD::V3 and HSAMD in local, it can pass for this error(just a test I don't think it's the right fix).

[foad]

@maurossi are you planning to commit this? Would you like me to commit it for you if you don't have access?

[maurossi]

Hello Jay,

sorry for late response,
being an external contributor I don't think I should have the rights to
apply commits in the official public llvm-project code base

Moreover in this moment I cannot even login via my google account,
thanks for having applied the patch

Mauro

llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

bool AMDGPUAsmParser::ParseDirectiveISAVersion() {
   return false;
 }
 
 bool AMDGPUAsmParser::ParseDirectiveHSAMetadata() {
   const char *AssemblerDirectiveBegin;
   const char *AssemblerDirectiveEnd;
   std::tie(AssemblerDirectiveBegin, AssemblerDirectiveEnd) =
       isHsaAbiVersion3AndAbove(&getSTI())
-          ? std::tuple(HSAMD::V3::AssemblerDirectiveBegin,
-                       HSAMD::V3::AssemblerDirectiveEnd)
-          : std::tuple(HSAMD::AssemblerDirectiveBegin,
-                       HSAMD::AssemblerDirectiveEnd);
+          ? std::pair(HSAMD::V3::AssemblerDirectiveBegin,
+          : std::pair(HSAMD::AssemblerDirectiveBegin,
 
   if (getSTI().getTargetTriple().getOS() != Triple::AMDHSA) {
     return Error(getLoc(),
                  (Twine(AssemblerDirectiveBegin) + Twine(" directive is "
                  "not available on non-amdhsa OSes")).str());
   }
 
   std::string HSAMetadataString;