Skip to content

Commit 025ed9f

Browse files
kripkenkripken
authored
[Branch Hinting] Fix Inlining's flipping of ifs, and add hint copying (#7714)
Split inlining will flip conditions in the copied code. To do this, add support for copying code annotations when we copy instructions (this is the first PR that can test that). This is done in a new metadata.h helper, that subsumes part of the previous debuginfo.h, that is, the new metadata::copyBetweenFunctions will copy all metadata, both debug info and code annotations.
1 parent f7c2851 commit 025ed9f

File tree

7 files changed

+143
-20
lines changed

7 files changed

+143
-20
lines changed

‎src/ir/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@ FILE(GLOB ir_HEADERS *.h)
22
set(ir_SOURCES
33
ExpressionAnalyzer.cpp
44
ExpressionManipulator.cpp
5-
debuginfo.cpp
65
drop.cpp
76
effects.cpp
87
eh-utils.cpp
98
export-utils.cpp
109
intrinsics.cpp
1110
lubs.cpp
1211
memory-utils.cpp
12+
metadata.cpp
1313
module-utils.cpp
1414
names.cpp
1515
possible-contents.cpp

‎src/ir/debuginfo.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,6 @@ inline void copyOriginalToReplacement(Expression* original,
6161
}
6262
}
6363

64-
// Given an expression and a copy of it in another function, copy the debug
65-
// info into the second function.
66-
void copyBetweenFunctions(Expression* origin,
67-
Expression* copy,
68-
Function* originFunc,
69-
Function* copyFunc);
7064
} // namespace wasm::debuginfo
7165

7266
#endif // wasm_ir_debuginfo_h

‎src/ir/debuginfo.cpp renamed to ‎src/ir/metadata.cpp

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,28 @@
1414
* limitations under the License.
1515
*/
1616

17-
#include "ir/debuginfo.h"
17+
#include "ir/metadata.h"
1818
#include "wasm-traversal.h"
1919
#include "wasm.h"
2020

21-
namespace wasm::debuginfo {
21+
namespace wasm::metadata {
2222

2323
void copyBetweenFunctions(Expression* origin,
2424
Expression* copy,
2525
Function* originFunc,
2626
Function* copyFunc) {
27-
if (originFunc->debugLocations.empty()) {
28-
return; // No debug info to copy
27+
if (originFunc->debugLocations.empty() &&
28+
originFunc->codeAnnotations.empty()) {
29+
// Nothing to copy.
30+
return;
2931
}
3032

33+
// List out instructions serially, so we can match them between the old and
34+
// new copies.
35+
//
36+
// This is not that efficient, and in theory we could copy this in the
37+
// caller context as the code is copied. However, we assume that most
38+
// functions have no metadata, so this is faster in that common case.
3139
struct Lister : public PostWalker<Lister, UnifiedExpressionVisitor<Lister>> {
3240
std::vector<Expression*> list;
3341
void visitExpression(Expression* curr) { list.push_back(curr); }
@@ -41,14 +49,25 @@ void copyBetweenFunctions(Expression* origin,
4149
auto& originDebug = originFunc->debugLocations;
4250
auto& copyDebug = copyFunc->debugLocations;
4351

52+
auto& originAnnotations = originFunc->codeAnnotations;
53+
auto& copyAnnotations = copyFunc->codeAnnotations;
54+
4455
assert(originList.list.size() == copyList.list.size());
4556
for (Index i = 0; i < originList.list.size(); i++) {
46-
auto iter = originDebug.find(originList.list[i]);
47-
if (iter != originDebug.end()) {
48-
auto location = iter->second;
49-
copyDebug[copyList.list[i]] = location;
57+
{
58+
auto iter = originDebug.find(originList.list[i]);
59+
if (iter != originDebug.end()) {
60+
copyDebug[copyList.list[i]] = iter->second;
61+
}
62+
}
63+
64+
{
65+
auto iter = originAnnotations.find(originList.list[i]);
66+
if (iter != originAnnotations.end()) {
67+
copyAnnotations[copyList.list[i]] = iter->second;
68+
}
5069
}
5170
}
5271
}
5372

54-
} // namespace wasm::debuginfo
73+
} // namespace wasm::metadata

‎src/ir/metadata.h

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*
2+
* Copyright 2019 WebAssembly Community Group participants
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
#ifndef wasm_ir_metadata_h
18+
#define wasm_ir_metadata_h
19+
20+
#include "wasm.h"
21+
22+
namespace wasm::metadata {
23+
24+
// Given an expression and a copy of it in another function, copy the all
25+
// metadata - debug info, code annotations - into the second function.
26+
void copyBetweenFunctions(Expression* origin,
27+
Expression* copy,
28+
Function* originFunc,
29+
Function* copyFunc);
30+
31+
} // namespace wasm::metadata
32+
33+
#endif // wasm_ir_metadata_h

‎src/ir/module-utils.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@
1515
*/
1616

1717
#include "module-utils.h"
18-
#include "ir/debuginfo.h"
1918
#include "ir/intrinsics.h"
2019
#include "ir/manipulation.h"
20+
#include "ir/metadata.h"
2121
#include "ir/properties.h"
2222
#include "support/insert_ordered.h"
2323
#include "support/topological_sort.h"
@@ -72,7 +72,7 @@ copyFunctionWithoutAdd(Function* func,
7272
ret->localNames = func->localNames;
7373
ret->localIndices = func->localIndices;
7474
ret->body = ExpressionManipulator::copy(func->body, out);
75-
debuginfo::copyBetweenFunctions(func->body, ret->body, func, ret.get());
75+
metadata::copyBetweenFunctions(func->body, ret->body, func, ret.get());
7676
ret->prologLocation = func->prologLocation;
7777
ret->epilogLocation = func->epilogLocation;
7878
// Update file indices if needed

‎src/passes/Inlining.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,15 @@
3030

3131
#include <atomic>
3232

33+
#include "ir/branch-hints.h"
3334
#include "ir/branch-utils.h"
34-
#include "ir/debuginfo.h"
3535
#include "ir/drop.h"
3636
#include "ir/eh-utils.h"
3737
#include "ir/element-utils.h"
3838
#include "ir/find_all.h"
3939
#include "ir/literal-utils.h"
4040
#include "ir/localize.h"
41+
#include "ir/metadata.h"
4142
#include "ir/module-utils.h"
4243
#include "ir/names.h"
4344
#include "ir/properties.h"
@@ -634,7 +635,7 @@ static void doCodeInlining(Module* module,
634635

635636
// Generate and update the inlined contents
636637
auto* contents = ExpressionManipulator::copy(from->body, *module);
637-
debuginfo::copyBetweenFunctions(from->body, contents, from, into);
638+
metadata::copyBetweenFunctions(from->body, contents, from, into);
638639
updater.walk(contents);
639640
block->list.push_back(contents);
640641
block->type = retType;
@@ -1097,6 +1098,7 @@ struct FunctionSplitter {
10971098
auto* inlineableIf = getIf(inlineable->body);
10981099
inlineableIf->condition =
10991100
builder.makeUnary(EqZInt32, inlineableIf->condition);
1101+
BranchHints::flip(inlineableIf, inlineable);
11001102
inlineableIf->ifTrue = builder.makeCall(
11011103
outlined->name, getForwardedArgs(func, builder), Type::none);
11021104
inlineable->body = inlineableIf;

‎test/lit/passes/inlining_splitting_branch-hints.wast

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
2+
3+
;; RUN: foreach %s %t wasm-opt --inlining --optimize-level=3 --partial-inlining-ifs=1 --all-features -S -o - | filecheck %s
4+
5+
;; The function we partially inline here has an if, which we emit as flipped
6+
;; afterwards. The new ifs should have flipped hints.
7+
8+
(module
9+
(func $func (param $0 i32)
10+
(@metadata.code.branch_hint "\01")
11+
(if
12+
(local.get $0)
13+
(then
14+
(return)
15+
)
16+
)
17+
;; More code, so this is not trivial.
18+
(loop $l
19+
(nop)
20+
)
21+
)
22+
;; CHECK: (type $0 (func))
23+
24+
;; CHECK: (type $1 (func (param i32)))
25+
26+
;; CHECK: (func $caller (type $0)
27+
;; CHECK-NEXT: (local $0 i32)
28+
;; CHECK-NEXT: (local $1 i32)
29+
;; CHECK-NEXT: (block $__inlined_func$byn-split-inlineable-A$func
30+
;; CHECK-NEXT: (local.set $0
31+
;; CHECK-NEXT: (i32.const 0)
32+
;; CHECK-NEXT: )
33+
;; CHECK-NEXT: (@metadata.code.branch_hint "\00")
34+
;; CHECK-NEXT: (if
35+
;; CHECK-NEXT: (i32.eqz
36+
;; CHECK-NEXT: (local.get $0)
37+
;; CHECK-NEXT: )
38+
;; CHECK-NEXT: (then
39+
;; CHECK-NEXT: (call $byn-split-outlined-A$func
40+
;; CHECK-NEXT: (local.get $0)
41+
;; CHECK-NEXT: )
42+
;; CHECK-NEXT: )
43+
;; CHECK-NEXT: )
44+
;; CHECK-NEXT: )
45+
;; CHECK-NEXT: (block $__inlined_func$byn-split-inlineable-A$func$1
46+
;; CHECK-NEXT: (local.set $1
47+
;; CHECK-NEXT: (i32.const 0)
48+
;; CHECK-NEXT: )
49+
;; CHECK-NEXT: (@metadata.code.branch_hint "\00")
50+
;; CHECK-NEXT: (if
51+
;; CHECK-NEXT: (i32.eqz
52+
;; CHECK-NEXT: (local.get $1)
53+
;; CHECK-NEXT: )
54+
;; CHECK-NEXT: (then
55+
;; CHECK-NEXT: (call $byn-split-outlined-A$func
56+
;; CHECK-NEXT: (local.get $1)
57+
;; CHECK-NEXT: )
58+
;; CHECK-NEXT: )
59+
;; CHECK-NEXT: )
60+
;; CHECK-NEXT: )
61+
;; CHECK-NEXT: )
62+
(func $caller
63+
(call $func
64+
(i32.const 0)
65+
)
66+
(call $func
67+
(i32.const 0)
68+
)
69+
)
70+
)
71+
;; CHECK: (func $byn-split-outlined-A$func (type $1) (param $0 i32)
72+
;; CHECK-NEXT: (loop $l
73+
;; CHECK-NEXT: (nop)
74+
;; CHECK-NEXT: )
75+
;; CHECK-NEXT: )

0 commit comments

Comments
 (0)