[PATCH] D142834: [X86][MC][bugfix] Report error for mismatched modifier in inline asm and remove function getX86SubSuperRegisterOrZero

[Kan Shengchen via Phabricator via llvm-commits]

skan added inline comments.


================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp:745
 
-MCRegister llvm::getX86SubSuperRegisterOrZero(MCRegister Reg, unsigned Size,
-                                              bool High) {
+MCRegister llvm::getX86SubSuperRegister(MCRegister Reg, unsigned Size,
+                                        bool High) {
----------------
craig.topper wrote:
> Didn't the old name more accurately describe this function?
I don't think so

1. `X86::NoRegister` is just a invalid value and it doesn't have to be zero
2. When we talk about a API like `getXXX`, we can expect it to return an invalid value, e.g  we use `getCondCode` rather than `getCondCodeOrInvalidCond`


================
Comment at: llvm/lib/Target/X86/X86AsmPrinter.cpp:545
   case 'b': // Print QImode register
     Reg = getX86SubSuperRegister(Reg, 8);
     break;
----------------
craig.topper wrote:
> Do we need to assert that Reg is valid? We used to have one.
https://llvm.org/docs/CodingStandards.html#assert-liberally 

`assert` is disabled for release build.  I belive it is only used to report internal error for LLVM developers and it should be used only when the developer has concern.
We have checked the register class of `Reg` at line 536-539, so `Reg` is always valid here obviously.  I think  `assertion` is no need here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142834