Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(92)

Side by Side Diff: sdk/lib/_internal/compiler/implementation/ssa/codegen_helpers.dart

Issue 196223017: Fix bad generate-at-use-site. (Closed) Base URL: https://dart.googlecode.com/svn/branches/bleeding_edge/dart
Patch Set: Address comments. Created 6 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch | Annotate | Revision Log
OLDNEW
1 // Copyright (c) 2012, the Dart project authors. Please see the AUTHORS file 1 // Copyright (c) 2012, the Dart project authors. Please see the AUTHORS file
2 // for details. All rights reserved. Use of this source code is governed by a 2 // for details. All rights reserved. Use of this source code is governed by a
3 // BSD-style license that can be found in the LICENSE file. 3 // BSD-style license that can be found in the LICENSE file.
4 4
5 part of ssa; 5 part of ssa;
6 6
7 /** 7 /**
8 * Replaces some instructions with specialized versions to make codegen easier. 8 * Replaces some instructions with specialized versions to make codegen easier.
9 * Caches codegen information on nodes. 9 * Caches codegen information on nodes.
10 */ 10 */
(...skipping 267 matching lines...) Expand 10 before | Expand all | Expand 10 after
278 for (HInstruction instruction = block.last.previous; 278 for (HInstruction instruction = block.last.previous;
279 instruction != null; 279 instruction != null;
280 instruction = instruction.previous) { 280 instruction = instruction.previous) {
281 if (generateAtUseSite.contains(instruction)) { 281 if (generateAtUseSite.contains(instruction)) {
282 continue; 282 continue;
283 } 283 }
284 if (instruction.isCodeMotionInvariant()) { 284 if (instruction.isCodeMotionInvariant()) {
285 markAsGenerateAtUseSite(instruction); 285 markAsGenerateAtUseSite(instruction);
286 continue; 286 continue;
287 } 287 }
288 if (instruction.isJsStatement()) {
289 expectedInputs.clear();
290 }
291 if (instruction.isPure()) { 288 if (instruction.isPure()) {
292 if (pureInputs.contains(instruction)) { 289 if (pureInputs.contains(instruction)) {
293 tryGenerateAtUseSite(instruction); 290 tryGenerateAtUseSite(instruction);
294 } else { 291 } else {
295 // If the input is not in the [pureInputs] set, it has not 292 // If the input is not in the [pureInputs] set, it has not
296 // been visited. 293 // been visited or should not be generated at use-site. The most
294 // likely reason for the latter, is that the instruction is used
295 // in more than one location.
296 // We must either clear the expectedInputs, or move the pure
297 // instruction's inputs in front of the existing ones.
298 // Example:
299 // t1 = foo(); // side-effect.
300 // t2 = bar(); // side-effect.
301 // t3 = pure(t2); // used more than once.
302 // f(t1, t3); // expected inputs of 'f': t1.
303 // use(t3);
304 //
305 // If we don't clear the expected inputs we end up in a situation
306 // where pure pushes "t2" on top of "t1" leading to:
307 // t3 = pure(bar());
308 // f(foo(), t3);
309 // use(t3);
310 //
311 // If we clear the expected-inputs list we have the correct
312 // output:
313 // t1 = foo();
314 // t3 = pure(bar());
315 // f(t1, t3);
316 // use(t3);
317 //
318 // Clearing is, however, not optimal.
319 // Example:
320 // t1 = foo(); // t1 is now used by `pure`.
321 // t2 = bar(); // t2 is now used by `f`.
322 // t3 = pure(t1);
323 // f(t2, t3);
324 // use(t3);
325 //
326 // If we clear the expected-inputs we can't generate-at-use any of
327 // the instructions.
328 //
329 // The optimal solution is to move the inputs of 'pure' in
330 // front of the expectedInputs list. This makes sense, since we
331 // push expected-inputs from left-to right, and the `pure` function
332 // invocation is "more left" (i.e. before) the first argument of `f`.
333 // With that approach we end up with:
334 // t3 = pure(foo();
335 // f(bar(), t3);
336 // use(t3);
337 //
338 int oldLength = expectedInputs.length;
297 instruction.accept(this); 339 instruction.accept(this);
340 if (oldLength != 0 && oldLength != expectedInputs.length) {
341 // Move the pure instruction's inputs to the front.
342 List<HInstruction> newInputs = expectedInputs.sublist(oldLength);
343 int newCount = newInputs.length;
344 expectedInputs.setRange(
345 newCount, newCount + oldLength, expectedInputs);
346 expectedInputs.setRange(0, newCount, newInputs);
347 }
298 } 348 }
299 } else { 349 } else {
300 if (findInInputsAndPopNonMatching(instruction)) { 350 if (findInInputsAndPopNonMatching(instruction)) {
301 // The current instruction is the next non-trivial 351 // The current instruction is the next non-trivial
302 // expected input. 352 // expected input.
303 tryGenerateAtUseSite(instruction); 353 tryGenerateAtUseSite(instruction);
304 } else { 354 } else {
305 assert(expectedInputs.isEmpty); 355 assert(expectedInputs.isEmpty);
306 } 356 }
307 instruction.accept(this); 357 instruction.accept(this);
(...skipping 176 matching lines...) Expand 10 before | Expand all | Expand 10 after
484 } 534 }
485 535
486 // If [thenInput] is defined in the first predecessor, then it is only used 536 // If [thenInput] is defined in the first predecessor, then it is only used
487 // by [phi] and can be generated at use site. 537 // by [phi] and can be generated at use site.
488 if (identical(thenInput.block, end.predecessors[0])) { 538 if (identical(thenInput.block, end.predecessors[0])) {
489 assert(thenInput.usedBy.length == 1); 539 assert(thenInput.usedBy.length == 1);
490 markAsGenerateAtUseSite(thenInput); 540 markAsGenerateAtUseSite(thenInput);
491 } 541 }
492 } 542 }
493 } 543 }
OLDNEW
« no previous file with comments | « sdk/lib/_internal/compiler/implementation/ssa/builder.dart ('k') | tests/language/pure_function2_test.dart » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698