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

Frédéric Riss friss at apple.com
Thu Nov 13 14:24:33 PST 2014


> On Nov 13, 2014, at 10:06 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Wed, Nov 12, 2014 at 6:34 PM, Frederic Riss <friss at apple.com <mailto: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.

There are like a dozen users, thus pushing the assert to the callers doesn’t seem like a good idea. I proposed removing the assert because  getDwarfRegnum() is so similar and doesn’t assert. If that weren’t the case, I would have implemented the 2 functions approach right away. I’ll just update the revision with a tryGetLLVMRegNum helper.

Fred 

> 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 <http://reviews.llvm.org/D6243>
> 
> Files:
>   lib/MC/MCRegisterInfo.cpp
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <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/4b7625f2/attachment.html>


More information about the llvm-commits mailing list