[PATCH] D102220: [NFC, CFI] Use unsigned type for CFI register values in parser/streamer code

[Venkata Ramanaiah Nalamothu via Phabricator via llvm-commits]

RamNalamothu added a comment.

In D102220#2755984 <https://reviews.llvm.org/D102220#2755984>, @MaskRay wrote:

> In D102220#2755883 <https://reviews.llvm.org/D102220#2755883>, @RamNalamothu wrote:
>
>> In D102220#2755545 <https://reviews.llvm.org/D102220#2755545>, @MaskRay wrote:
>>
>>>> the interfacing code in parsers and streamers are using both signed and unsigned type in different parts.
>>>
>>> Where is uint64_t used? Most places use int64_t.
>>
>> For instance in `AsmParser::parseRegisterOrRegisterNumber()` and type cast in `MCStreamer::emitCFIDefCfaRegister()`.



  bool AsmParser::parseRegisterOrRegisterNumber(int64_t &Register,          ---> signed for register
                                                SMLoc DirectiveLoc) {
    unsigned RegNo;                                                         ---> unsigned for register
  
    if (getLexer().isNot(AsmToken::Integer)) {
      if (getTargetParser().ParseRegister(RegNo, DirectiveLoc, DirectiveLoc))
        return true;
      Register = getContext().getRegisterInfo()->getDwarfRegNum(RegNo, true);
    } else
      return parseAbsoluteExpression(Register);
  
    return false;
  }



>> Anyway, I have changed the summary as the above scenarios are minor in the entire changes.
>
> Both `AsmParser::parseRegisterOrRegisterNumber()` and `MCStreamer::emitCFIDefCfaRegister()` uses int64_t.
>
> OK, I am now convinced that most places use int64_t.
> Using unsigned type to represent "negative values aren't allowed" is not a good idea.

Looks like I couldn't get the intent of this patch expressed better earlier.

This patch is based on the fact that we don't use negative numbers to represent registers in CFI, meaning DWARF register mappings always use positive numbers or at least that is what I understood. Based on that, if not using register names in CFI directives, we never emit

`.cfi_register -2020, 2050`       ---> value of register `-2020` is saved in register `2050`.

So, does it make sense to use an unsigned type for a variable which is not expected to have negative numbers?
And that is what this patch is trying to achieve.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102220