[PATCH] Change MCRegisterInfo::getLLVMRegNum() not to assert on unknown register.

[David Blaikie]

On Wed, Nov 12, 2014 at 6:34 PM, Frederic Riss <friss at apple.com> wrote:

> I want to use this method in llvm-dwarfdump to be able to pretty
> print register names in dwarf expressions. As the data I'll feed
> to the method comes directly from the binary file, it might be
> anything and I'd like to avoid asserting when an unknown register
> number is requested.
>
> This patch makes the method return -1 when the register isn't found.
> The new behavior is the same as its inverse function getDwarfRegNum.
> Note that although this method returns an 'int', each and every user I
> could find stores the result in an 'unsigned' variable. Would it
> make sense to change the method return type to unsigned? Should the
> assert be kept and propagated to the callers? Any other suggestion
> to achieve what I want?
>

I think it depnds on how many callers there are - if there's one or two
(ish), then just putting an "assert(result != -1)" in the callers is
probably sufficient. If there's a fair few, it might make sense to preserve
this function as asserting on failure, and extract its implementation into
another function ("tryGetLLVMRegNum"?) that returns -1 (or whatever) on
failure - the original getLLVMRegNum would call this implementation, assert
!= -1, and return.

If you were to change the return type to unsigned, what would you return on
failure for your use case? If we have just one function (and push an assert
into existing callers) I'd leave it signed and update callers to use signed
values, assert, etc... if we end up with two functions I think it's fine
for get* to return unsigned and tryGet* to return int.

- David


> http://reviews.llvm.org/D6243
>
> Files:
>   lib/MC/MCRegisterInfo.cpp
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141113/a3d0f682/attachment.html>