[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:748
   switch (Size) {
   default: return X86::NoRegister;
   case 8:
----------------
pengfei wrote:
> skan wrote:
> > pengfei wrote:
> > > skan wrote:
> > > > skan wrote:
> > > > > craig.topper wrote:
> > > > > > Why do we allow invalid sizes?
> > > > > I think the previous author was overly conservative here. Let's use a `llvm_unreachable` here.
> > > > > Why do we allow invalid sizes?
> > > > 
> > > > The test  `CodeGen/X86/asm-reject-reg-type-mismatch.ll` will fail if we use `llvm_unreachable` here.
> > > > 
> > > > ```
> > > > ; CHECK: error: couldn't allocate output register for constraint '{ax}'
> > > > define i128 @blup() {
> > > >   %v = tail call i128 asm "", "={ax},0"(i128 0)
> > > >   ret i128 %v
> > > > }
> > > > ```
> > > > 
> > > > See more details here https://reviews.llvm.org/D10813 
> > > Is it caused by error message not match?
> > > Is it caused by error message not match?
> > 
> > It is caused by the crash due to the `llvm_unreachable` at line 748.
> > 
> > ```
> > UNREACHABLE executed at llvm-project/llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp:748!
> > 
> > ```
> > `getX86SubSuperRegister` is called with `Size`  128, and an inline asm error is emitted in `SelectionDAGBuilder.cpp` when the returned register is not valid.
> > If we used "llvm_unreachable" at line 748, the llc would crash before emitting an error.
> > 
> > 
> Will it be solved by adding `--crash` after `not`?
Yes, we also need to move other tests in `asm-reject-reg-type-mismatch.ll` into another file

```
; CHECK: error: couldn't allocate input reg for constraint 'r'
define void @fp80(x86_fp80) {
  tail call void asm sideeffect "", "r"(x86_fp80 %0)
  ret void
}

; CHECK: error: couldn't allocate input reg for constraint 'f'
define void @f_constraint_i128(ptr %0) {
  %2 = load i128, ptr %0, align 16
  tail call void asm sideeffect "", "f"(i128 %2)
  ret void
}

```

b/c when llc crashes, the remaining file will not be processed. But, should we do such thing?

I think we can just remove the function `blup` and the check in this test b/c it is impossible case. Does it sound good to you?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142834