llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.09k stars 11.6k forks source link

longjmp intrinsic silently dropped on AArch64 compilations #55141

Open ayazhafiz opened 2 years ago

ayazhafiz commented 2 years ago

I have the following IR:

target triple = "aarch64-darwin-macos"

@l_sjlj_buffer = global [40 x i8] zeroinitializer

; Function Attrs: nobuiltin nounwind
define void @l_panic(i8* nonnull %0, i32 %1) local_unnamed_addr {
entry:
  store i8* %0, i8** bitcast (i8* getelementptr inbounds ([40 x i8], [40 x i8]* @l_sjlj_buffer, i32 0, i64 24) to i8**), align 8
  call void @llvm.eh.sjlj.longjmp(i8* getelementptr inbounds ([40 x i8], [40 x i8]* @l_sjlj_buffer, i32 0, i32 0))
  unreachable
}

; Function Attrs: noreturn nounwind
declare void @llvm.eh.sjlj.longjmp(i8*)

For which d42f222f9d6f01afe99d0d597c98b36eb205c5ec generates the following code (via llc, without error):

    .section    __TEXT,__text,regular,pure_instructions
    .globl  _l_panic                        ; -- Begin function l_panic
    .p2align    2
_l_panic:                               ; @l_panic
    .cfi_startproc
; %bb.0:                                ; %entry
Lloh0:
    adrp    x8, _l_sjlj_buffer@PAGE
Lloh1:
    add x8, x8, _l_sjlj_buffer@PAGEOFF
Lloh2:
    str x0, [x8, #24]
    .loh AdrpAddStr Lloh0, Lloh1, Lloh2
    .cfi_endproc
                                        ; -- End function
    .globl  _l_sjlj_buffer                  ; @l_sjlj_buffer
.zerofill __DATA,__common,_l_sjlj_buffer,40,4
.subsections_via_symbols

Note that there is no branch after the store under Lloh2. I ran llc -debug on this code, which indicates that the AArch64 backend does not expand the longjmp intrinsic and instead the instruction is dropped in LegalizeDAG:

Type-legalized selection DAG: %bb.0 'l_panic:entry'
SelectionDAG has 9 nodes:
  t0: ch = EntryToken
      t2: i64,ch = CopyFromReg t0, Register:i64 %0
      t7: i64 = add nuw GlobalAddress:i64<[40 x i8]* @l_sjlj_buffer> 0, Constant:i64<24>
    t10: ch = store<(store (s64) into `i8** bitcast (i8* getelementptr inbounds ([40 x i8], [40 x i8]* @l_sjlj_buffer, i32 0, i64 24) to i8**)`)> t0, t2, t7, undef:i64
  t11: ch = EH_SJLJ_LONGJMP t10, GlobalAddress:i64<[40 x i8]* @l_sjlj_buffer> 0

Legalizing: t11: ch = EH_SJLJ_LONGJMP t10, GlobalAddress:i64<[40 x i8]* @l_sjlj_buffer> 0
Trying to expand node
Successfully expanded node
 ... replacing: t11: ch = EH_SJLJ_LONGJMP t10, GlobalAddress:i64<[40 x i8]* @l_sjlj_buffer> 0
     with:      t10: ch = store<(store (s64) into `i8** bitcast (i8* getelementptr inbounds ([40 x i8], [40 x i8]* @l_sjlj_buffer, i32 0, i64 24) to i8**)`)> t0, t2, t7, undef:i64

If the AArch64 backend does not support the intrinsic, can a failure be issued rather than dropping the instruction? Of note, opt --verify did not issue a warning here, and I see elsewhere that clang issues a failure if __builtin_longjmp is used in C code targeting AArch64.

I am an LLVM novice, so I might also just be doing something wrong here. Do let me know if this is the case.

llvmbot commented 2 years ago

@llvm/issue-subscribers-backend-aarch64

DavidSpickett commented 2 years ago

I can't explain the behaviour but I looked around a bit, might help. Your use of llc looks fine to me.

When targetting AArch64 SelectionDAGLegalize::LegalizeOp figures out what to do with the longjmp.

  case ISD::EH_SJLJ_SETJMP:
  case ISD::EH_SJLJ_LONGJMP:
  case ISD::EH_SJLJ_SETUP_DISPATCH:
    // These operations lie about being legal: when they claim to be legal,
    // they should actually be expanded.
    Action = TLI.getOperationAction(Node->getOpcode(), Node->getValueType(0));
    if (Action == TargetLowering::Legal)
      Action = TargetLowering::Expand;
    break;

This tells us to Expand the longjmp (why this is backwards I don't know), so later we call ExpandNode which says:

  case ISD::EH_SJLJ_LONGJMP:
    // If the target didn't expand these, there's nothing to do, so just
    // preserve the chain and be done.
    Results.push_back(Node->getOperand(0));
    break;

So the expansion "succeeds" and we don't attempt to fallthrough to a LibCall.

I think the logic here is that setjmp/longjmp are target specific enough to need custom lowering per target, it can't be done in a generic way. This is what ARM does (documented here https://llvm.org/docs/CodeGenerator.html#selectiondag-legalize-phase).

It is odd that only these few node types seem to do this replacement. With a quick scan I only saw one that asserts:

case ISD::FMAD:
    llvm_unreachable("Illegal fmad should never be formed");

For some things dropping them makes sense, like ISD::PREFETCH which is a hint. For setjmp/longjmp I don't see how that would be valid to do.

If I just comment out EH_SJLJ_LONGJMP from the expandNode switch I get this targeting AArch64:

===== Instruction selection begins: %bb.0 'entry'

ISEL: Starting selection on root node: t11: ch = EH_SJLJ_LONGJMP t10, t13
ISEL: Starting pattern match
  Match failed at index 0
LLVM ERROR: Cannot select: t11: ch = EH_SJLJ_LONGJMP t10, t13
  t13: i64 = AArch64ISD::LOADgot TargetGlobalAddress:i64<[40 x i8]* @l_sjlj_buffer> 0 [TF=16]
    t12: i64 = TargetGlobalAddress<[40 x i8]* @l_sjlj_buffer> 0 [TF=16]

Which seems like what you want. If I choose x86 or Arm it's custom lowered as before.

fhahn commented 2 years ago

@TNorthover do you by any chance have any thoughts on this one?