[PATCH] D143120: [llvm] infer rtlib from triple for codegen decisions

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: llvm/include/llvm/TargetParser/Triple.h:296
+    LIB_GCC,
+    WINDOWS_RT,
+  };
----------------
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`.


================
Comment at: llvm/lib/TargetParser/Triple.cpp:1235
+  return COMPILER_RT;
+}
+
----------------
mstorsjo wrote:
> 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.
Thanks - the change here seems to take care of my immediate concerns at least.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143120/new/

https://reviews.llvm.org/D143120