[X86] Improve diagnostics for named registers

[e-kud]

Summary

Closes #62984

[e-kud]

Rebased

[e-kud]

Hi @pengfei @RKSimon, what do you think about such diagnostic improvement? It works perfectly with clang, however llc ignores errors in a handler and continues execution. It seems that we need a combination of diagnostic and report_fatal_error to stop both clang and llc. Or is it expected behavior of llc?

[pengfei]

I think the expected behavior is to diagnostic this problem from the front end. I didn't look into details, but guess you may be able to do it in SemaChecking.cpp.

[pengfei]

Maybe the obstacle is we cannot get FP infor at the time.

[e-kud]

In D152332#4489290, @pengfei wrote:

I think the expected behavior is to diagnostic this problem from the front end. I didn't look into details, but guess you may be able to do it in SemaChecking.cpp.

In D152332#4489303, @pengfei wrote:

Maybe the obstacle is we cannot get FP infor at the time.

Generally the problem is that front end is too early for the diagnostic.

E.g. for the following two functions we should get the diagnostic only for the second. Because the first function will emit frame pointer and accessing rbp is valid. Also some optimizations may influence on presence of the frame pointer.

int register x __asm("ebp");
int square0(int num) {
    int y[num];
    bar(y);
    return x * num;
}

int register x __asm("ebp");
int square1(int num) {
    return x * num;
}

BUT, we still can handle it in front end. We may be more strict: if a global register variable on the frame pointer register is used in a statement, we should check -fno-omit-frame-pointer applied to statement's context. -fno-omit-frame-pointer is a guarantee that variable can be on rbp register.

I actually have no idea how to implement it in FE due to unfamiliarity with it. BTW, AArch64 simply miscompiles without any error.

Abandon revision for the better times

llvm/lib/Target/X86/X86ISelLowering.cpp

SDValue X86TargetLowering::LowerFRAMEADDR(SDValue Op, SelectionDAG &DAG) const {
   return FrameAddr;
 }
 
 // FIXME? Maybe this could be a TableGen attribute on some registers and
 // this table could be generated automatically from RegInfo.
 Register X86TargetLowering::getRegisterByName(const char* RegName, LLT VT,
                                               const MachineFunction &MF) const {
   const TargetFrameLowering &TFI = *Subtarget.getFrameLowering();
+  LLVMContext &Ctx = MF.getFunction().getContext();
 
   Register Reg = StringSwitch<unsigned>(RegName)
                        .Case("esp", X86::ESP)
                        .Case("rsp", X86::RSP)
                        .Case("ebp", X86::EBP)
                        .Case("rbp", X86::RBP)
                        .Default(0);
 
   if (Reg == X86::EBP || Reg == X86::RBP) {
     if (!TFI.hasFP(MF))
-      report_fatal_error("register " + StringRef(RegName) +
-                         " is allocatable: function has no frame pointer");
+      Ctx.emitError("register '" + StringRef(RegName) +
+                    "' is allocatable: function '" + MF.getName() +
+                    "' has no frame pointer");
 #ifndef NDEBUG
     else {
       const X86RegisterInfo *RegInfo = Subtarget.getRegisterInfo();
       Register FrameReg = RegInfo->getPtrSizedFrameRegister(MF);
       assert((FrameReg == X86::EBP || FrameReg == X86::RBP) &&
              "Invalid Frame Register!");
     }
 #endif
   }
 
   if (Reg)
     return Reg;
 
-  report_fatal_error("Invalid register name global variable");
+  Ctx.emitError("invalid register name for a global variable: " +
+                StringRef(RegName));
+  return Reg;
 }
 
 SDValue X86TargetLowering::LowerFRAME_TO_ARGS_OFFSET(SDValue Op,
                                                      SelectionDAG &DAG) const {
   const X86RegisterInfo *RegInfo = Subtarget.getRegisterInfo();
   return DAG.getIntPtrConstant(2 * RegInfo->getSlotSize(), SDLoc(Op));
 }
 

llvm/test/CodeGen/X86/named-reg-alloc.ll

 ; RUN: not --crash llc < %s -mtriple=x86_64-apple-darwin 2>&1 | FileCheck %s
 ; RUN: not --crash llc < %s -mtriple=x86_64-linux-gnueabi 2>&1 | FileCheck %s
 
 define i32 @get_stack() nounwind {
 entry:
 ; FIXME: Include an allocatable-specific error message
-; CHECK: Invalid register name global variable
-  %sp = call i32 @llvm.read_register.i32(metadata !0)
+; CHECK: error: invalid register name for a global variable: eax
+  %sp = call i32 @llvm.read_register.i32(metadata !0)
   ret i32 %sp
 }
 
 declare i32 @llvm.read_register.i32(metadata) nounwind
 
 !0 = !{!"eax\00"}

llvm/test/CodeGen/X86/named-reg-notareg.ll

 ; RUN: not --crash llc < %s -mtriple=x86_64-apple-darwin 2>&1 | FileCheck %s
 ; RUN: not --crash llc < %s -mtriple=x86_64-linux-gnueabi 2>&1 | FileCheck %s
 
 define i32 @get_stack() nounwind {
 entry:
-; CHECK: Invalid register name global variable
-  %sp = call i32 @llvm.read_register.i32(metadata !0)
+; CHECK: error: invalid register name for a global variable: notareg
+  %sp = call i32 @llvm.read_register.i32(metadata !0)
   ret i32 %sp
 }
 
 declare i32 @llvm.read_register.i32(metadata) nounwind
 
 !0 = !{!"notareg\00"}

llvm/test/CodeGen/X86/read-fp-no-frame-pointer.ll

-; RUN: not --crash llc < %s -mtriple=x86_64-linux-gnueabi 2>&1 | FileCheck %s
+; RUN: not llc < %s -mtriple=x86_64-linux-gnueabi 2>&1 | FileCheck %s
 
 define i32 @get_frame() nounwind {
 entry:
-; CHECK: register ebp is allocatable: function has no frame pointer
+; CHECK: error: register 'ebp' is allocatable: function 'get_frame' has no frame pointer
   %fp = call i32 @llvm.read_register.i32(metadata !0)
   ret i32 %fp
 }
 
 declare i32 @llvm.read_register.i32(metadata) nounwind
 
 !0 = !{!"ebp\00"}