Function has token parameter but isn't an intrinsic #40056

compnerd · 2019-02-13T00:41:00Z


Bugzilla Link	40710
Version	unspecified
OS	Windows NT
CC	@DougGregor,@hiraditya,@pcc,@zygoloid,@rnk,@smeenai,@vedantk

Extended Description

Reduced from ICU 61:

// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc19.0.0 -emit-obj -o /dev/null -O1 -fcxx-exceptions -fexceptions %s
struct T {
  virtual ~T();
};

void f();

struct S {
  virtual ~S();
  T t_[1];
};

S::~S() { f(); }

compnerd · 2019-02-13T00:41:00Z

assigned to @hiraditya

compnerd · 2019-02-13T00:46:48Z

The IR verifier might be too stringent on the token checking. The outlined filter funclet is given a token to tie it to the parent clause but is a "Function" in IR land. This triggers a verification failure.

compnerd · 2019-02-13T01:03:52Z

This is caused by the hot/cold splitting code path promoting the token to a formal argument.

rnk · 2019-02-13T01:25:30Z

(pasting in stuff I said in IRC)

outlining WinEH code is fine, but unlikely to be profitable or save size, as EH funclets are already "outlined", so it's a bit redundant
if you do outline code with funclet bundles, just delete all "funclet" bundle operands and make a "funclet" operand bundle on the call to the outlined code

And, I think the verifier is working as intended here. You are not supposed to be able to pass a token as a regular parameter, it can't be passed through registers/memory/phis, etc, and I think arguments fit in there as well. It just so happens that you can look at the bundle type ("funclet" in this case) and reason about it, so it is possible to do what you want, but in the general case, I don't think tokens can be outlined.

vedantk · 2019-02-13T19:52:37Z

(pasting in stuff I said in IRC)

outlining WinEH code is fine, but unlikely to be profitable or save size,
as EH funclets are already "outlined", so it's a bit redundant

I only have a vague understanding of WinEH, so apologies for the naive question: is the funclet really the thing being split here?

It looks like two blocks dominated by a cleanuppad are split, and afaik they aren't normally outlined?

if you do outline code with funclet bundles, just delete all "funclet"
bundle operands and make a "funclet" operand bundle on the call to the
outlined code

Should splitting even be allowed on functions with WinEH personality? I don't think any run-time testing has been done with this. And as you point out, it may not be profitable.

And, I think the verifier is working as intended here. You are not supposed
to be able to pass a token as a regular parameter, it can't be passed
through registers/memory/phis, etc, and I think arguments fit in there as
well. It just so happens that you can look at the bundle type ("funclet" in
this case) and reason about it, so it is possible to do what you want, but
in the general case, I don't think tokens can be outlined.

This points to a separate bug. CodeExtractor shouldn't permit this.

vedantk · 2019-02-13T20:28:53Z

Proposed patch (disable splitting within functions with scoped EH personalities)

compnerd · 2019-02-13T20:46:29Z

Created attachment 21477 [details]
Proposed patch (disable splitting within functions with scoped EH
personalities)

I think that we should still teach the Hot/Cold splitting to deal with the tokens. But, yes the funclets should not be split into the cold path since they are already outlined.

rnk · 2019-02-13T21:08:55Z

I only have a vague understanding of WinEH, so apologies for the naive
question: is the funclet really the thing being split here?

It looks like two blocks dominated by a cleanuppad are split, and afaik they
aren't normally outlined?

When I compile this code with -O2 and run the hotcoldsplit pass on it, I see this block being outlined:

Found a cold block:

ehcleanup:                                        ; preds = %entry
  %1 = cleanuppad within none []
  %arraydestroy.element7 = getelementptr inbounds %struct.S, %struct.S* %this, i64 0, i32 1, i64 0
  call void @"??1T@@UEAA@XZ"(%struct.T* nonnull %arraydestroy.element7) #&#8203;4 [ "funclet"(token %1) ]
  call void @&#8203;__std_terminate() #&#8203;5 [ "funclet"(token %1) ]
  unreachable

Outlining in ??_GS@@UEAAPEAXI@Z

The blocks in the cleanuppad region constitute the funclet. They share a stack frame with the parent function, but the code is discontiguous from the parent function and has its own prologue/epilogue.

Should splitting even be allowed on functions with WinEH personality? I
don't think any run-time testing has been done with this. And as you point
out, it may not be profitable.

Up to you. I think if we update CodeExtractor to be token aware, this may sort itself out naturally. Keep in mind that checking isScopedEHPersonality also affects web assembly, which I think uses the same representation.

And, I think the verifier is working as intended here. You are not supposed
to be able to pass a token as a regular parameter, it can't be passed
through registers/memory/phis, etc, and I think arguments fit in there as
well. It just so happens that you can look at the bundle type ("funclet" in
this case) and reason about it, so it is possible to do what you want, but
in the general case, I don't think tokens can be outlined.

This points to a separate bug. CodeExtractor shouldn't permit this.

Makes sense.

vedantk · 2019-02-13T21:22:25Z

I only have a vague understanding of WinEH, so apologies for the naive
question: is the funclet really the thing being split here?

It looks like two blocks dominated by a cleanuppad are split, and afaik they
aren't normally outlined?

When I compile this code with -O2 and run the hotcoldsplit pass on it, I see
this block being outlined:

'''
Found a cold block:

ehcleanup: ; preds = %entry
%1 = cleanuppad within none []
%arraydestroy.element7 = getelementptr inbounds %struct.S, %struct.S*
%this, i64 0, i32 1, i64 0
call void @"??1T@@UEAA@XZ"(%struct.T* nonnull %arraydestroy.element7) #4 [
"funclet"(token %1) ]
call void @__std_terminate() #5 [ "funclet"(token %1) ]
unreachable

Outlining in ??_GS@@UEAAPEAXI@Z
'''

The blocks in the cleanuppad region constitute the funclet. They share a
stack frame with the parent function, but the code is discontiguous from the
parent function and has its own prologue/epilogue.

The splitting pass will refuse to extract EH pads (see mayExtractBlock). It does still consider EH pads to be cold, though, and will propagate that information.

By 'cleanuppad region', can I assume you mean blocks which use a cleannuppad token?

Should splitting even be allowed on functions with WinEH personality? I
don't think any run-time testing has been done with this. And as you point
out, it may not be profitable.

Up to you. I think if we update CodeExtractor to be token aware, this may
sort itself out naturally. Keep in mind that checking isScopedEHPersonality
also affects web assembly, which I think uses the same representation.

I'll take a cut at this, but might need some help with the run-time testing bit.

And, I think the verifier is working as intended here. You are not supposed
to be able to pass a token as a regular parameter, it can't be passed
through registers/memory/phis, etc, and I think arguments fit in there as
well. It just so happens that you can look at the bundle type ("funclet" in
this case) and reason about it, so it is possible to do what you want, but
in the general case, I don't think tokens can be outlined.

This points to a separate bug. CodeExtractor shouldn't permit this.

Makes sense.

compnerd · 2019-02-13T21:38:09Z

BTW, I ended up with this reduction:

; RUN: opt -hotcoldsplit %s -S -o /dev/null

define void @"??1S@@UEAA@XZ"() personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*) {
entry:
  invoke void @"?f@@YAXXZ"()
          to label %invoke.cont unwind label %ehcleanup

invoke.cont:
  ret void

ehcleanup:
  %0 = cleanuppad within none []
  br label %ehcleanup.resume

ehcleanup.resume:
  call void @"?f@@YAXXZ"() [ "funclet"(token %0) ]
  br label %terminate

terminmate:
  call void @"?terminate@@YAXXZ"() [ "funclet"(token %0) ]
  unreachable
}

declare void @"?f@@YAXXZ"()

declare i32 @__CxxFrameHandler3(...)

declare void @"?terminate@@YAXXZ"()

hiraditya · 2019-09-24T21:30:55Z

Thanks for providing the repro @Saleem. There's a small typo so the fixed up repro is here:

; RUN: opt --hot-cold-split %s -S -o /dev/null

define void @"??1S@@UEAA@XZ"() personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*) {
entry:
  invoke void @"?f@@YAXXZ"()
          to label %invoke.cont unwind label %ehcleanup

invoke.cont:
  ret void

ehcleanup:
  %0 = cleanuppad within none []
  br label %ehcleanup.resume

ehcleanup.resume:
  call void @"?f@@YAXXZ"() [ "funclet"(token %0) ]
  br label %terminate

terminate:
  call void @"?terminate@@YAXXZ"() [ "funclet"(token %0) ]
  unreachable
}

declare void @"?f@@YAXXZ"()

declare i32 @__CxxFrameHandler3(...)

declare void @"?terminate@@YAXXZ"()

the compilation aborts after hotcoldsplit with the following IR.

*** IR Dump After Hot Cold Splitting ***
; ModuleID = '<stdin>'
source_filename = "<stdin>"

define void @"??1S@@UEAA@XZ"() personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*) {
entry:
  invoke void @"?f@@YAXXZ"()
          to label %invoke.cont unwind label %ehcleanup

invoke.cont:                                      ; preds = %entry
  ret void

ehcleanup:                                        ; preds = %entry
  %0 = cleanuppad within none []
  br label %codeRepl

codeRepl:                                         ; preds = %ehcleanup
  call void @"??1S@@[email protected]"(token %0) #1
  ret void
}

declare void @"?f@@YAXXZ"()

declare i32 @__CxxFrameHandler3(...)

declare void @"?terminate@@YAXXZ"()

; Function Attrs: cold minsize noreturn
define internal void @"??1S@@[email protected]"(token %0) #0 personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*) {
newFuncRoot:
  br label %ehcleanup.resume

ehcleanup.resume:                                 ; preds = %newFuncRoot
  call void @"?f@@YAXXZ"() [ "funclet"(token %0) ]
  br label %terminate

terminate:                                        ; preds = %ehcleanup.resume
  call void @"?terminate@@YAXXZ"() [ "funclet"(token %0) ]
  unreachable
}

attributes #0 = { cold minsize noreturn }
attributes #1 = { noinline }
Function has token parameter but isn't an intrinsic
  call void @"??1S@@[email protected]"(token %0) #1
in function ??1S@@UEAA@XZ
LLVM ERROR: Broken function found, compilation aborted!

hiraditya · 2019-09-29T14:01:25Z

Bug fix: D68192

hiraditya · 2023-03-23T22:03:30Z

Attaching the patch by @vedantk(#40056 (comment)) above.
0001-HotColdSplit-Disable-splitting-within-functions-with.txt

swift-inspect will fail to build in release mode after the dump-arrays PR (swiftlang/swift#66973) due to an llvm bug (llvm/llvm-project#40056). This is a workaround for it.

compnerd · 2024-07-20T02:34:31Z

@Endilll - what exactly needs to be done in terms of triaging? This is an issue with the hot cold splitting implementation, which is done in LLVM.

hiraditya · 2024-07-20T13:20:22Z

I think the fix is to disable outlining of basic blocks with token-type instructions

…lvm#99759) Hot cold splitting should not outline: 1. Basic blocks with token type instructions 1. Functions with scoped EH personality (As suggested by Vedant in llvm#40056 (comment)) Fixes: llvm#40056 (cherry picked from commit c59ee7e)

…lvm#99759) Hot cold splitting should not outline: 1. Basic blocks with token type instructions 1. Functions with scoped EH personality (As suggested by Vedant in llvm#40056 (comment)) Fixes: llvm#40056 (cherry picked from commit c59ee7e) (cherry picked from commit 267c6ed)

…99759) Hot cold splitting should not outline: 1. Basic blocks with token type instructions 1. Functions with scoped EH personality (As suggested by Vedant in #40056 (comment)) Fixes: #40056

Adds some flags for Swift's LLVM compiler to succeed on Windows. Related: llvm/llvm-project#40056 Signed-off-by: kingbri <[email protected]>

llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021

compnerd mentioned this issue May 24, 2022

build: workaround LLVM bug on Windows bazelbuild/rules_swift#831

Merged

stevapple mentioned this issue Sep 20, 2022

llbuildSwift fails to build with SwiftPM release configuration on Windows x86_64 swiftlang/swift-llbuild#844

Open

hjyamauchi mentioned this issue Jul 3, 2023

Add a build workaround for the swift-inspect release build compnerd/swift-build#587

Merged

Endilll added the clang:to-be-triaged Should not be used for new issues label Jul 19, 2024

Endilll added llvm Umbrella label for LLVM issues and removed clang:to-be-triaged Should not be used for new issues labels Jul 20, 2024

hiraditya mentioned this issue Jul 20, 2024

Fix 40056: Prevent outlining of blocks with token type instructions #99759

Merged

hiraditya closed this as completed in #99759 Jul 20, 2024

hiraditya closed this as completed in c59ee7e Jul 20, 2024

EugeneZelenko removed c++ llvm Umbrella label for LLVM issues labels Jul 20, 2024

EugeneZelenko added the ipo Interprocedural optimizations label Jul 20, 2024

compnerd mentioned this issue Jul 23, 2024

Fix 40056: Prevent outlining of blocks with token type instructions (… swiftlang/llvm-project#9000

Merged

compnerd mentioned this issue Jul 24, 2024

Fix 40056: Prevent outlining of blocks with token type instructions (… swiftlang/llvm-project#9005

Merged

kingbri1 added a commit to kingbri1/llama.cpp that referenced this issue Jul 29, 2024

Swift: Fix Windows build

aac0c69

Adds some flags for Swift's LLVM compiler to succeed on Windows. Related: llvm/llvm-project#40056 Signed-off-by: kingbri <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Function has token parameter but isn't an intrinsic #40056

Function has token parameter but isn't an intrinsic #40056

compnerd commented Feb 13, 2019 •

edited by Endilll

Loading

compnerd commented Feb 13, 2019

compnerd commented Feb 13, 2019

compnerd commented Feb 13, 2019

rnk commented Feb 13, 2019

vedantk commented Feb 13, 2019

vedantk commented Feb 13, 2019

compnerd commented Feb 13, 2019

rnk commented Feb 13, 2019 •

edited by Endilll

Loading

vedantk commented Feb 13, 2019 •

edited by Endilll

Loading

compnerd commented Feb 13, 2019 •

edited by Endilll

Loading

hiraditya commented Sep 24, 2019 •

edited by Endilll

Loading

hiraditya commented Sep 29, 2019

hiraditya commented Mar 23, 2023

compnerd commented Jul 20, 2024

hiraditya commented Jul 20, 2024

Function has token parameter but isn't an intrinsic #40056

Function has token parameter but isn't an intrinsic #40056

Comments

compnerd commented Feb 13, 2019 • edited by Endilll Loading

Extended Description

compnerd commented Feb 13, 2019

compnerd commented Feb 13, 2019

compnerd commented Feb 13, 2019

rnk commented Feb 13, 2019

vedantk commented Feb 13, 2019

vedantk commented Feb 13, 2019

compnerd commented Feb 13, 2019

rnk commented Feb 13, 2019 • edited by Endilll Loading

vedantk commented Feb 13, 2019 • edited by Endilll Loading

compnerd commented Feb 13, 2019 • edited by Endilll Loading

hiraditya commented Sep 24, 2019 • edited by Endilll Loading

hiraditya commented Sep 29, 2019

hiraditya commented Mar 23, 2023

compnerd commented Jul 20, 2024

hiraditya commented Jul 20, 2024

compnerd commented Feb 13, 2019 •

edited by Endilll

Loading

rnk commented Feb 13, 2019 •

edited by Endilll

Loading

vedantk commented Feb 13, 2019 •

edited by Endilll

Loading

compnerd commented Feb 13, 2019 •

edited by Endilll

Loading

hiraditya commented Sep 24, 2019 •

edited by Endilll

Loading