[ELF] Correctly set the nvptx triple from makeTriple()
#76970
Conversation
Summary: The ELFObject file should be able to handle `nvptx` objects be we currently list them as unknown. This patch should now make it return `nvptx64--` correctly.
jhuber6
requested review from
Artem-B,
EthanLuisMcDonough,
jdoerfert,
jlebar,
JonChesterfield and
MaskRay
|
@llvm/pr-subscribers-llvm-binary-utilities Author: Joseph Huber (jhuber6) ChangesSummary: Full diff: https://github.com/llvm/llvm-project/pull/76970.diff 1 Files Affected:
diff --git a/llvm/include/llvm/Object/ELFObjectFile.h b/llvm/include/llvm/Object/ELFObjectFile.h
index 99477644de4de7..da78e11b678d99 100644
--- a/llvm/include/llvm/Object/ELFObjectFile.h
+++ b/llvm/include/llvm/Object/ELFObjectFile.h
@@ -1349,6 +1349,13 @@ template <class ELFT> Triple::ArchType ELFObjectFile<ELFT>::getArch() const {
return Triple::UnknownArch;
}
+ case ELF::EM_CUDA: {
+ if (EF.getHeader().e_ident[ELF::EI_CLASS] == ELF::ELFCLASS32)
+ return Triple::nvptx;
+ else
+ return Triple::nvptx64;
+ }
+
case ELF::EM_BPF:
return IsLittleEndian ? Triple::bpfel : Triple::bpfeb;
|
Typo? |
| if (EF.getHeader().e_ident[ELF::EI_CLASS] == ELF::ELFCLASS32) | ||
| return Triple::nvptx; | ||
| else | ||
| return Triple::nvptx64; |
There was a problem hiding this comment.
avoid else-after-return, per the LLVM style guide.
Instead use:
if (...)
return ...;
return ...;
There was a problem hiding this comment.
There's a test for this now in #76992. I can do the style NFC if needed.
There was a problem hiding this comment.
Generally the test should go with the patch that introduces the change in behavior. If the change in behavior is unreachable/untestable without some other changes, then they should go together so test coverage is clear (otherwise it's hard to track that the patch did eventually get tested).
Sure - an NFC change would be good - no need to send it for review, you can commit that directly.
There was a problem hiding this comment.
There weren't any existing tests for any of this code, so I just punted it until the second patch. Thanks, I'll do the NFC.
|
Test coverage? |
Summary:
The ELFObject file should be able to handle
nvptxobjects but wecurrently list them as unknown. This patch should now make it return
nvptx64--correctly.