[PATCH] D111358: TargetLibraryInfo checker tool

[Paul Robinson via Phabricator via llvm-commits]

probinson added a comment.

Sorry for the long delay, between CPPcon and putting together a dev-meeting talk I've been pretty distracted.

In D111358#3055417 <https://reviews.llvm.org/D111358#3055417>, @rengolin wrote:

> The actual code looks super clean and easy to read, except for the `SeparateMode` part (to me, totally personal).
>
> Perhaps it would be simpler if you always use an array of reports, and in separate mode, you populate the array one by one, but on joint mode you append to the first item, then report the array, however many items are there?

Yeah, that part is the bit I'm least happy about, it kind of just grew and probably could use a refactor now.
I invented separate mode so I could tell which library files had any libcalls in them at all, and reduce the fileset that I needed to look at.  Maybe instead of separate mode, there could be a summary mode, that gave just the total number of matches per file.  That would provide the functionality I actually needed.



================
Comment at: llvm/docs/CommandGuide/llvm-tli-checker.rst:50
+  A base directory to prepend to each library file path. This is handy
+  when there are a number of library files all in the same directory.
+
----------------
rengolin wrote:
> So, the syntax:
> 
>     llvm-tli-checker ... --libdir=foo/bar a.o b.o c.o
> 
> will check against:
>  * foo/bar/a.o
>  * foo/bar/b.o
>  * foo/bar/c.o
> 
> If so, couldn't this just be `foo/bar/*` and let the shell expand it?
That could work.  In my use case, the list of files to check is fixed, but I wanted to check against various releases, so it was easiest to put the list of files in a response file and then do `--libdir=foo/bar @myliblist.txt` for each release directory.
Also each foo/bar has many more files than just the libc/libm equivalents, and it's faster to look only at the specific files I want, so wildcarding would cost time and generate noise in the output.
I'd prefer to keep the option; I'll add a note about the response-file use case.


================
Comment at: llvm/docs/CommandGuide/llvm-tli-checker.rst:65
+  Print information for each library call known to TLI, instead of just the
+  mismatches.
+
----------------
rengolin wrote:
> This feels like more of a separate functionality, for example, `--list`, then just being verbose.
> 
> I usually interpret `verbose` as showing the steps taken, not showing more information.
> 
> Instead, if I just want to list the symbols (for ex. for grepping), then do I need to create an empty object file and use `--verbose`?
My inspiration was llvm-dwarfdump, where the default display shows you (for example) that an entity has a name attribute, and what that name is. In verbose mode, it also shows you details of the encoding, how it found the name.  In that case, verbose does mean more information, so I did the same here.
Perhaps "print information" doesn't describe the behavior well, maybe "print results" is better?  As it prints the results of all checks, not just the ones that don't match.
I could also see adding a `--list` option that didn't look at object files, but just dumped the info from TLI, would that be useful?


================
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
----------------
rengolin wrote:
> `llvm-tli-checker`?
Oops


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

https://reviews.llvm.org/D111358