[llvm] r251113 - [CodeGen] Mark setjmp/catchret MBBs address-taken

Quentin Colombet via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 11 16:47:36 PST 2015


Hi Joseph,

With shrink-wrapping enabled, the test case added in this commit fails and I need your feedback on the output (see attachments).

The new output seems incorrect to me and assuming it is, could you explain what we should check to teach shrink-wrapping to do the right thing?
Alternatively, it may be possible it is just that we miss a proper expansion for CATCHRET.
Anyway, please advise.

Thanks,
Q.
 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: withoutShrinkWrapping.s
Type: application/octet-stream
Size: 3470 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151111/2ff94e0b/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: withShrinkWrapping.s
Type: application/octet-stream
Size: 3282 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151111/2ff94e0b/attachment-0001.obj>
-------------- next part --------------

> On Oct 23, 2015, at 8:06 AM, Joseph Tremoulet via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: josepht
> Date: Fri Oct 23 10:06:05 2015
> New Revision: 251113
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=251113&view=rev
> Log:
> [CodeGen] Mark setjmp/catchret MBBs address-taken
> 
> Summary:
> This ensures that BranchFolding (and similar) won't remove these blocks.
> 
> Also allow AsmPrinter::EmitBasicBlockStart to process MBBs which are
> address-taken but do not have BBs that are address-taken, since otherwise
> its call to getAddrLabelSymbolTableToEmit would fail an assertion on such
> blocks.  I audited the other callers of getAddrLabelSymbolTableToEmit
> (and getAddrLabelSymbol); they all have BBs known to be address-taken
> except for the call through getAddrLabelSymbol from
> WinException::create32bitRef; that call is actually now unreachable, so
> I've removed it and updated the signature of create32bitRef.
> 
> This fixes PR25168.
> 
> Reviewers: majnemer, andrew.w.kaylor, rnk
> 
> Subscribers: pgavlin, llvm-commits
> 
> Differential Revision: http://reviews.llvm.org/D13774
> 
> Added:
>    llvm/trunk/test/CodeGen/X86/late-address-taken.ll
> Modified:
>    llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
>    llvm/trunk/lib/CodeGen/AsmPrinter/WinException.cpp
>    llvm/trunk/lib/CodeGen/AsmPrinter/WinException.h
>    llvm/trunk/lib/Target/X86/X86FrameLowering.cpp
>    llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
>    llvm/trunk/test/CodeGen/X86/win-catchpad-csrs.ll
>    llvm/trunk/test/CodeGen/X86/win-catchpad-nested.ll
>    llvm/trunk/test/CodeGen/X86/win-catchpad.ll
> 
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp?rev=251113&r1=251112&r2=251113&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp Fri Oct 23 10:06:05 2015
> @@ -2480,8 +2480,11 @@ void AsmPrinter::EmitBasicBlockStart(con
>     if (isVerbose())
>       OutStreamer->AddComment("Block address taken");
> 
> -    for (MCSymbol *Sym : MMI->getAddrLabelSymbolToEmit(BB))
> -      OutStreamer->EmitLabel(Sym);
> +    // MBBs can have their address taken as part of CodeGen without having
> +    // their corresponding BB's address taken in IR
> +    if (BB->hasAddressTaken())
> +      for (MCSymbol *Sym : MMI->getAddrLabelSymbolToEmit(BB))
> +        OutStreamer->EmitLabel(Sym);
>   }
> 
>   // Print some verbose block comments.
> 
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/WinException.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/WinException.cpp?rev=251113&r1=251112&r2=251113&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/WinException.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/WinException.cpp Fri Oct 23 10:06:05 2015
> @@ -273,12 +273,10 @@ const MCExpr *WinException::create32bitR
>                                  Asm->OutContext);
> }
> 
> -const MCExpr *WinException::create32bitRef(const Value *V) {
> -  if (!V)
> +const MCExpr *WinException::create32bitRef(const GlobalValue *GV) {
> +  if (!GV)
>     return MCConstantExpr::create(0, Asm->OutContext);
> -  if (const auto *GV = dyn_cast<GlobalValue>(V))
> -    return create32bitRef(Asm->getSymbol(GV));
> -  return create32bitRef(MMI->getAddrLabelSymbol(cast<BasicBlock>(V)));
> +  return create32bitRef(Asm->getSymbol(GV));
> }
> 
> const MCExpr *WinException::getLabelPlusOne(const MCSymbol *Label) {
> 
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/WinException.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/WinException.h?rev=251113&r1=251112&r2=251113&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/WinException.h (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/WinException.h Fri Oct 23 10:06:05 2015
> @@ -18,6 +18,7 @@
> 
> namespace llvm {
> class Function;
> +class GlobalValue;
> class MachineFunction;
> class MCExpr;
> class Value;
> @@ -66,7 +67,7 @@ class LLVM_LIBRARY_VISIBILITY WinExcepti
>                                      StringRef FLinkageName);
> 
>   const MCExpr *create32bitRef(const MCSymbol *Value);
> -  const MCExpr *create32bitRef(const Value *V);
> +  const MCExpr *create32bitRef(const GlobalValue *GV);
>   const MCExpr *getLabelPlusOne(const MCSymbol *Label);
>   const MCExpr *getOffset(const MCSymbol *OffsetOf, const MCSymbol *OffsetFrom);
>   const MCExpr *getOffsetPlusOne(const MCSymbol *OffsetOf,
> 
> Modified: llvm/trunk/lib/Target/X86/X86FrameLowering.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86FrameLowering.cpp?rev=251113&r1=251112&r2=251113&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/X86FrameLowering.cpp (original)
> +++ llvm/trunk/lib/Target/X86/X86FrameLowering.cpp Fri Oct 23 10:06:05 2015
> @@ -1178,6 +1178,9 @@ void X86FrameLowering::emitEpilogue(Mach
>           .addReg(ReturnReg)
>           .addMBB(RestoreMBB);
>     }
> +    // Record that we've taken the address of RestoreMBB and no longer just
> +    // reference it in a terminator.
> +    RestoreMBB->setHasAddressTaken();
>   }
> 
>   if (MBBI != MBB.end())
> 
> Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=251113&r1=251112&r2=251113&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
> +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Fri Oct 23 10:06:05 2015
> @@ -21471,7 +21471,7 @@ X86TargetLowering::emitEHSjLjSetJmp(Mach
>   // For v = setjmp(buf), we generate
>   //
>   // thisMBB:
> -  //  buf[LabelOffset] = restoreMBB
> +  //  buf[LabelOffset] = restoreMBB <-- takes address of restoreMBB
>   //  SjLjSetup restoreMBB
>   //
>   // mainMBB:
> @@ -21491,6 +21491,7 @@ X86TargetLowering::emitEHSjLjSetJmp(Mach
>   MF->insert(I, mainMBB);
>   MF->insert(I, sinkMBB);
>   MF->push_back(restoreMBB);
> +  restoreMBB->setHasAddressTaken();
> 
>   MachineInstrBuilder MIB;
> 
> 
> Added: llvm/trunk/test/CodeGen/X86/late-address-taken.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/late-address-taken.ll?rev=251113&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/late-address-taken.ll (added)
> +++ llvm/trunk/test/CodeGen/X86/late-address-taken.ll Fri Oct 23 10:06:05 2015
> @@ -0,0 +1,68 @@
> +; RUN: llc -mtriple=x86_64-pc-windows-msvc < %s | FileCheck %s
> +
> +; Repro cases from PR25168
> +
> +; test @catchret - catchret target is not address-taken until PEI
> +; splits it into lea/mov followed by ret.  Make sure the MBB is
> +; handled, both by tempting BranchFolding to merge it with %early_out
> +; and delete it, and by checking that we emit a proper reference
> +; to it in the LEA
> +
> +declare void @ProcessCLRException()
> +declare void @f()
> +
> +define void @catchret(i1 %b) personality void ()* @ProcessCLRException {
> +entry:
> +  br i1 %b, label %body, label %early_out
> +early_out:
> +  ret void
> +body:
> +  invoke void @f()
> +          to label %exit unwind label %catch.pad
> +catch.pad:
> +  %catch = catchpad [i32 33554467]
> +          to label %catch.body unwind label %catch.end
> +catch.body:
> +  catchret %catch to label %exit
> +catch.end:
> +  catchendpad unwind to caller
> +exit:
> +  ret void
> +}
> +; CHECK-LABEL: catchret:  # @catchret
> +; CHECK: [[Exit:^[^ :]+]]: # Block address taken
> +; CHECK-NEXT:              # %exit
> +; CHECK: # %catch.pad
> +; CHECK: .seh_endprolog
> +; CHECK: leaq [[Exit]](%rip), %rax
> +; CHECK: retq # CATCHRET
> +
> +
> +; test @setjmp - similar to @catchret, but the MBB in question
> +; is the one generated when the setjmp's block is split
> +
> + at buf = internal global [5 x i8*] zeroinitializer
> +declare i8* @llvm.frameaddress(i32) nounwind readnone
> +declare i8* @llvm.stacksave() nounwind
> +declare i32 @llvm.eh.sjlj.setjmp(i8*) nounwind
> +declare void @llvm.eh.sjlj.longjmp(i8*) nounwind
> +
> +define void @setjmp(i1 %b) nounwind {
> +entry:
> +  br i1 %b, label %early_out, label %sj
> +early_out:
> +  ret void
> +sj:
> +  %fp = call i8* @llvm.frameaddress(i32 0)
> +  store i8* %fp, i8** getelementptr inbounds ([5 x i8*], [5 x i8*]* @buf, i64 0, i64 0), align 16
> +  %sp = call i8* @llvm.stacksave()
> +  store i8* %sp, i8** getelementptr inbounds ([5 x i8*], [5 x i8*]* @buf, i64 0, i64 2), align 16
> +  call i32 @llvm.eh.sjlj.setjmp(i8* bitcast ([5 x i8*]* @buf to i8*))
> +  ret void
> +}
> +; CHECK-LABEL: setjmp: # @setjmp
> +; CHECK: # %sj
> +; CHECK: leaq [[Label:\..+]](%rip), %[[Reg:.+]]{{$}}
> +; CHECK-NEXT: movq %[[Reg]], buf
> +; CHECK: {{^}}[[Label]]:  # Block address taken
> +; CHECK-NEXT:              # %sj
> 
> Modified: llvm/trunk/test/CodeGen/X86/win-catchpad-csrs.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/win-catchpad-csrs.ll?rev=251113&r1=251112&r2=251113&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/win-catchpad-csrs.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/win-catchpad-csrs.ll Fri Oct 23 10:06:05 2015
> @@ -113,7 +113,8 @@ catchendblock:
> ; X64: callq useints
> ; X64: movl $1, %ecx
> ; X64: callq f
> -; X64: [[contbb:\.LBB0_[0-9]+]]: # %try.cont
> +; X64: [[contbb:\.LBB0_[0-9]+]]: # Block address taken
> +; X64-NEXT:                      # %try.cont
> ; X64: addq $40, %rsp
> ; X64: popq %rbp
> ; X64: retq
> @@ -188,7 +189,8 @@ catchendblock:
> ; X64: callq useints
> ; X64: movl $1, %ecx
> ; X64: callq f
> -; X64: [[contbb:\.LBB1_[0-9]+]]: # %try.cont
> +; X64: [[contbb:\.LBB1_[0-9]+]]: # Block address taken
> +; X64-NEXT:                      # %try.cont
> ; X64: addq $40, %rsp
> ; X64-NOT: popq
> ; X64: popq %rsi
> @@ -249,7 +251,8 @@ catchendblock:
> ; X64: .seh_endprologue
> ; X64: movl $1, %ecx
> ; X64: callq f
> -; X64: [[contbb:\.LBB2_[0-9]+]]: # %try.cont
> +; X64: [[contbb:\.LBB2_[0-9]+]]: # Block address taken
> +; X64-NEXT:                      # %try.cont
> ; X64: addq $48, %rsp
> ; X64-NOT: popq
> ; X64: popq %rbp
> 
> Modified: llvm/trunk/test/CodeGen/X86/win-catchpad-nested.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/win-catchpad-nested.ll?rev=251113&r1=251112&r2=251113&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/win-catchpad-nested.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/win-catchpad-nested.ll Fri Oct 23 10:06:05 2015
> @@ -31,8 +31,10 @@ exit:
> 
> ; Check the catchret targets
> ; CHECK-LABEL: test1: # @test1
> -; CHECK: [[Exit:^[^: ]+]]: # %exit
> -; CHECK: [[OuterRet:^[^: ]+]]: # %outer.ret
> +; CHECK: [[Exit:^[^: ]+]]: # Block address taken
> +; CHECK-NEXT:              # %exit
> +; CHECK: [[OuterRet:^[^: ]+]]: # Block address taken
> +; CHECK-NEXT:                  # %outer.ret
> ; CHECK-NEXT: leaq [[Exit]](%rip), %rax
> ; CHECK:      retq   # CATCHRET
> ; CHECK: {{^[^: ]+}}: # %inner.pad
> 
> Modified: llvm/trunk/test/CodeGen/X86/win-catchpad.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/win-catchpad.ll?rev=251113&r1=251112&r2=251113&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/win-catchpad.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/win-catchpad.ll Fri Oct 23 10:06:05 2015
> @@ -74,13 +74,15 @@ catchendblock:
> ; X86: [[contbb:LBB0_[0-9]+]]: # %try.cont
> ; X86: retl
> 
> -; X86: [[restorebb1:LBB0_[0-9]+]]: # %invoke.cont.2
> +; X86: [[restorebb1:LBB0_[0-9]+]]: # Block address taken
> +; X86-NEXT:                        # %invoke.cont.2
> ; X86: movl -16(%ebp), %esp
> ; X86: addl $12, %ebp
> ; X86: jmp [[contbb]]
> 
> ; FIXME: These should be de-duplicated.
> -; X86: [[restorebb2:LBB0_[0-9]+]]: # %invoke.cont.3
> +; X86: [[restorebb2:LBB0_[0-9]+]]: # Block address taken
> +; X86-NEXT:                        # %invoke.cont.3
> ; X86: movl -16(%ebp), %esp
> ; X86: addl $12, %ebp
> ; X86: jmp [[contbb]]
> @@ -140,7 +142,8 @@ catchendblock:
> ; X64-DAG: leaq -[[local_offs:[0-9]+]](%rbp), %rdx
> ; X64-DAG: movl $1, %ecx
> ; X64: callq f
> -; X64: [[contbb:\.LBB0_[0-9]+]]: # %try.cont
> +; X64: [[contbb:\.LBB0_[0-9]+]]: # Block address taken
> +; X64-NEXT:                      # %try.cont
> ; X64: addq $48, %rsp
> ; X64: popq %rbp
> ; X64: retq
> @@ -253,7 +256,8 @@ catchendblock:
> ; X86: [[contbb:LBB1_[0-9]+]]: # %try.cont
> ; X86: retl
> 
> -; X86: [[restorebb:LBB1_[0-9]+]]: # %catch.done
> +; X86: [[restorebb:LBB1_[0-9]+]]: # Block address taken
> +; X86-NEXT:                       # %catch.done
> ; X86: movl -16(%ebp), %esp
> ; X86: addl $12, %ebp
> ; X86: jmp [[contbb]]
> @@ -294,7 +298,8 @@ catchendblock:
> ; X64: .Ltmp[[before_call:[0-9]+]]:
> ; X64: callq f
> ; X64: .Ltmp[[after_call:[0-9]+]]:
> -; X64: [[contbb:\.LBB1_[0-9]+]]: # %try.cont
> +; X64: [[contbb:\.LBB1_[0-9]+]]: # Block address taken
> +; X64-NEXT:                      # %try.cont
> ; X64: addq $48, %rsp
> ; X64: popq %rbp
> ; X64: retq
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list