[PATCH] D26648: Clarify semantic of reserved registers.

Quentin Colombet via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 30 13:24:12 PST 2016


> 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.

> 
> 
> Repository:
>  rL LLVM
> 
> https://reviews.llvm.org/D26648
> 
> 
> 



More information about the llvm-commits mailing list