[llvm] infer rtlib from triple for codegen decisions
Changes PlannedPublic
Actions

Authored by nickdesaulniers on Feb 1 2023, 4:23 PM.

Details

Summary

To run unittests:

$ cd llvm/build; ninja UnitTests; cd -
$ ./llvm/build/unittests/TargetParser/TargetParserTests \
    --gtest_filter=TargetParserTest.getRTLib

	Time	Test
	70 ms	x64 debian > LLVM.CodeGen/X86::muloti.ll Script: -- : 'RUN: at line 2'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/llc < /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/X86/muloti.ll -mtriple=x86_64-apple-darwin \| /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/X86/muloti.ll
	140 ms	x64 debian > LLVM.CodeGen/X86::overflow-intrinsic-optimizations.ll Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/llc /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/X86/overflow-intrinsic-optimizations.ll -mtriple=i386-linux-gnu -o - \| /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/X86/overflow-intrinsic-optimizations.ll --check-prefix=CHECK-LIBGCC
	90 ms	x64 debian > LLVM.CodeGen/X86::smulo-128-legalisation-lowering.ll Script: -- : 'RUN: at line 2'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/llc -mtriple=x86_64-darwin-unknown < /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/X86/smulo-128-legalisation-lowering.ll \| /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/llvm/test/CodeGen/X86/smulo-128-legalisation-lowering.ll --check-prefix=X64

nickdesaulniers requested review of this revision.Feb 1 2023, 4:23 PM

nickdesaulniers retitled this revision from alternative to https://reviews.llvm.org/D108928 to [llvm.Feb 1 2023, 4:26 PM

nickdesaulniers retitled this revision from [llvm to [llvm] infer rtlib from triple for codegen decisions.

nickdesaulniers edited the summary of this revision. (Show Details)

mstorsjo added inline comments.

llvm/lib/TargetParser/Triple.cpp
1235	Depending on the actual use of this function, I think we'd need to be more careful than this, to only return a non-unknown value for the platforms where we're absolutely certain that there can't be any other choice. For platforms like apple and android, this probably is the case - possibly for some of the BSDs as well (not entirely sure). At least for mingw targets (which do hit the `isGNUEnvironment()` at the top), toolchains with both libgcc and compiler-rt are in active use. With the only use of `getRTLib()` as suggested in this patch, it's harmless to return libgcc (it'd be equal to returning unknown), but with the function in place, the function may be used in other ways too.

llvm/include/llvm/TargetParser/Triple.h
296	Some comments on the naming here... There's really nothing like `WINDOWS_RT` - the OS itself doesn't have anything corresponding to it, but it's all a toolchain specific library - I think the most correct name for it right now would be `VCRUNTIME`. Also as a nitpick/personal opinion, spelling it `LIB_GCC` feels weird to me, I'd just spell it `LIBGCC`.
llvm/lib/TargetParser/Triple.cpp
1235	Thanks - the change here seems to take care of my immediate concerns at least.

llvm/include/llvm/TargetParser/Triple.h
296	Another suggestion on the naming... we don't use `ALL_CAPS` for other enum values unless they're acronyms, so `CompilerRT`, `Libgcc` and `VCRuntime` would be more in line with the existing values.