[PATCH] D26648: Clarify semantic of reserved registers.

[Matthias Braun via llvm-commits]

> On Nov 30, 2016, at 1:24 PM, Quentin Colombet via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
>> 
>> On Nov 30, 2016, at 12:48 PM, Matthias Braun via Phabricator <reviews at reviews.llvm.org> wrote:
>> 
>> MatzeB added inline comments.
>> 
>> 
>> ================
>> Comment at: include/llvm/Target/TargetRegisterInfo.h:950
>> +  /// of the set as well.
>> +  void assertAllSuperRegsMarked(const BitVector &RegisterSet,
>> +      ArrayRef<MCPhysReg> Exceptions = ArrayRef<MCPhysReg>()) const;
>> ----------------
>> qcolombet wrote:
>>> I'd change the API a bit:
>>> - Rename in checkAllSuperRegsMarked
>>> - Return bool
>>> - Have the users assert on that bool.
>>> 
>>> The rationale is that in release build `assert(checkAllSuperRegsMarked)` can be optimized out, but `assertAllSuperRegsMarked` would still cause a call to an empty function.
>> Actually my first version returned a bool and had the `assert()` on the caller. I changed my mind because I wanted the display a message listing the register causing the problem and writing to stderr in a check function felt wrong. Would you think writing to stderr is fine anyway?
> 
> Maybe in dbgs if we keep the message here.
> 
>> Or should I just get rid of the message and have people figure out the problematic register in a debugger?
> 
> Alternatively, we could provide a utility function to call from the debugger to get that printed when we hit the assertion.
> 
> Up to you.
I'll got with the first variant and print to dbgs(). Thanks.

- Matthias
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161130/88ce7b86/attachment.html>