[PATCH] D111358: TargetLibraryInfo checker tool

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/docs/CommandGuide/llvm-tli-checker.rst:74
+
+:program:`llvm-cxxfilt` returns 0 even if there are mismatches. It returns a
+non-zero exit code if there is an unrecognized option, or no input files are
----------------
Copy-paste error :)


================
Comment at: llvm/test/tools/llvm-tli-checker/ps4-tli-check.s:8
+# RUN: llvm-mc --triple=x86_64-scei-ps4 --defsym WRONG=1 --filetype=obj %s -o %t2.o
+# RUN: ld.lld --shared %t2.o -o %t2.so
+# RUN: echo %t2.so > %t2.txt
----------------
MaskRay wrote:
> The relanded version (38be8f4057c1bf19fd02d08d6116e28983a49d8d) uses prebuilt shared objects `Inputs/ps4-tli-check.right.so` and `Inputs/ps4-tli-check.wrong.so`. For llvm/tools/* tools working on object files, we try very hard to avoid checking in new prebuilt files: they are opaque, difficult to update, and waste space.
> 
> It's recommended to use yaml2obj, like many tests in test/tools/llvm-objcopy and test/tools/llvm-readobj.
> 
> @jhenderson 
+1 to what MaskRay said. If you need guidance on what the YAML might look like, feel free to chat with me outside this review.

You also can't use `lld` in a RUN command outside the `lld` and `cross-project-tests` projects, since there's no guarantee it would have been built (`llvm/tools` tests only have access to the core `llvm` project components).

Apologies for not noticing these points earlier: this slipped off my radar, and I ended up looking at other stuff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111358