Skip to content

Commit 9269973

Browse files
alexmalyshevfacebook-github-bot
authored andcommitted
Fix lir::Rewrite using std::forward twice on the same value
Summary: Happens to work today because the value is always copied, but it's sketchy. Reviewed By: yoney Differential Revision: D81813071 fbshipit-source-id: b5a32d805409a6cab37fc3167c82c6e5e0872edc
1 parent fe516d3 commit 9269973

4 files changed

Lines changed: 31 additions & 20 deletions

File tree

cinderx/Jit/lir/postalloc.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "cinderx/Jit/codegen/x86_64.h"
66
#include "cinderx/Jit/containers.h"
77
#include "cinderx/Jit/jit_rt.h"
8+
#include "cinderx/Jit/lir/function.h"
89
#include "cinderx/Jit/lir/operand.h"
910

1011
#include <optional>

cinderx/Jit/lir/postgen.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include "cinderx/Jit/lir/postgen.h"
44

55
#include "cinderx/Jit/lir/inliner.h"
6+
#include "cinderx/Jit/lir/printer.h"
67

78
using namespace jit::codegen;
89

cinderx/Jit/lir/rewrite.cpp

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,33 @@
22

33
#include "cinderx/Jit/lir/rewrite.h"
44

5-
#include "cinderx/Common/log.h"
65
#include "cinderx/Jit/lir/block.h"
6+
#include "cinderx/Jit/lir/function.h"
77
#include "cinderx/Jit/lir/operand.h"
88

99
#include <set>
1010

1111
namespace jit::lir {
1212

13+
Rewrite::Rewrite(Function* func, codegen::Environ* env)
14+
: function_(func), env_(env) {}
15+
16+
Function* Rewrite::function() {
17+
return function_;
18+
}
19+
20+
const Function* Rewrite::function() const {
21+
return function_;
22+
}
23+
24+
codegen::Environ* Rewrite::environment() {
25+
return env_;
26+
}
27+
28+
const codegen::Environ* Rewrite::environment() const {
29+
return env_;
30+
}
31+
1332
void Rewrite::run() {
1433
// collect all stages
1534
std::set<int> stages;

cinderx/Jit/lir/rewrite.h

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,8 @@
55
#include "cinderx/Common/concepts.h"
66
#include "cinderx/Jit/codegen/environ.h"
77
#include "cinderx/Jit/lir/block.h"
8-
#include "cinderx/Jit/lir/printer.h"
98

109
#include <functional>
11-
#include <memory>
1210
#include <type_traits>
1311
#include <utility>
1412
#include <vector>
@@ -42,23 +40,15 @@ using instruction_rewrite_t = function_type_t<instruction_rewrite_arg_t>;
4240
// This class implements a framework for LIR rewrites.
4341
class Rewrite {
4442
public:
45-
Rewrite(Function* func, codegen::Environ* env) : function_(func), env_(env) {}
43+
Rewrite(Function* func, codegen::Environ* env);
4644

47-
Function* function() {
48-
return function_;
49-
}
50-
51-
const Function* function() const {
52-
return function_;
53-
}
45+
// Get the function being rewritten.
46+
Function* function();
47+
const Function* function() const;
5448

55-
codegen::Environ* environment() {
56-
return env_;
57-
}
58-
59-
const codegen::Environ* environment() const {
60-
return env_;
61-
}
49+
// Get the codegen environment.
50+
codegen::Environ* environment();
51+
const codegen::Environ* environment() const;
6252

6353
template <typename T>
6454
void registerOneRewriteFunction(RewriteResult (*rewrite)(T), int stage = 0) {
@@ -114,14 +104,14 @@ class Rewrite {
114104
template <
115105
AnyOf<function_rewrite_t, basic_block_rewrite_t, instruction_rewrite_t> T,
116106
typename V>
117-
bool runOneTypeRewrites(const std::vector<T>& rewrites, V&& arg) {
107+
bool runOneTypeRewrites(const std::vector<T>& rewrites, const V& arg) {
118108
bool changed = false;
119109
bool loop_changed = false;
120110

121111
do {
122112
loop_changed = false;
123113
for (auto& rewrite : rewrites) {
124-
auto r = rewrite(std::forward<V>(arg));
114+
auto r = rewrite(arg);
125115
loop_changed |= (r != kUnchanged);
126116

127117
if (r == kRemoved) {

0 commit comments

Comments
 (0)