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

Side by Side Diff: pkg/compiler/lib/src/ssa/codegen_helpers.dart

Issue 1913843003: Revert "Fix for issue 26243 - illegal motion of assignments in instruction merging" (Closed) Base URL: https://github.com/dart-lang/sdk.git@master
Patch Set: Created 4 years, 7 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
« no previous file with comments | « no previous file | pkg/compiler/lib/src/ssa/optimize.dart » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 import '../compiler.dart' show Compiler; 5 import '../compiler.dart' show Compiler;
6 import '../constants/values.dart'; 6 import '../constants/values.dart';
7 import '../elements/elements.dart'; 7 import '../elements/elements.dart';
8 import '../js_backend/js_backend.dart'; 8 import '../js_backend/js_backend.dart';
9 import '../types/types.dart'; 9 import '../types/types.dart';
10 import '../universe/selector.dart' show Selector; 10 import '../universe/selector.dart' show Selector;
(...skipping 335 matching lines...) Expand 10 before | Expand all | Expand 10 after
346 void analyzeInputs(HInstruction user, int start) { 346 void analyzeInputs(HInstruction user, int start) {
347 List<HInstruction> inputs = user.inputs; 347 List<HInstruction> inputs = user.inputs;
348 for (int i = start; i < inputs.length; i++) { 348 for (int i = start; i < inputs.length; i++) {
349 HInstruction input = inputs[i]; 349 HInstruction input = inputs[i];
350 if (!generateAtUseSite.contains(input) && 350 if (!generateAtUseSite.contains(input) &&
351 !input.isCodeMotionInvariant() && 351 !input.isCodeMotionInvariant() &&
352 input.usedBy.length == 1 && 352 input.usedBy.length == 1 &&
353 input is! HPhi && 353 input is! HPhi &&
354 input is! HLocalValue && 354 input is! HLocalValue &&
355 !input.isJsStatement()) { 355 !input.isJsStatement()) {
356 if (isEffectivelyPure(input)) { 356 if (input.isPure()) {
357 // Only consider a pure input if it is in the same loop. 357 // Only consider a pure input if it is in the same loop.
358 // Otherwise, we might move GVN'ed instruction back into the 358 // Otherwise, we might move GVN'ed instruction back into the
359 // loop. 359 // loop.
360 if (user.hasSameLoopHeaderAs(input)) { 360 if (user.hasSameLoopHeaderAs(input)) {
361 // Move it closer to [user], so that instructions in 361 // Move it closer to [user], so that instructions in
362 // between do not prevent making it generate at use site. 362 // between do not prevent making it generate at use site.
363 input.moveBefore(user); 363 input.moveBefore(user);
364 pureInputs.add(input); 364 pureInputs.add(input);
365 // Previous computations done on [input] are now invalid 365 // Previous computations done on [input] are now invalid
366 // because we moved [input] to another place. So all 366 // because we moved [input] to another place. So all
367 // non code motion invariant instructions need 367 // non code motion invariant instructions need
368 // to be removed from the [generateAtUseSite] set. 368 // to be removed from the [generateAtUseSite] set.
369 input.inputs.forEach((instruction) { 369 input.inputs.forEach((instruction) {
370 if (!instruction.isCodeMotionInvariant()) { 370 if (!instruction.isCodeMotionInvariant()) {
371 generateAtUseSite.remove(instruction); 371 generateAtUseSite.remove(instruction);
372 } 372 }
373 }); 373 });
374 // Visit the pure input now so that the expected inputs 374 // Visit the pure input now so that the expected inputs
375 // are after the expected inputs of [user]. 375 // are after the expected inputs of [user].
376 input.accept(this); 376 input.accept(this);
377 } 377 }
378 } else { 378 } else {
379 expectedInputs.add(input); 379 expectedInputs.add(input);
380 } 380 }
381 } 381 }
382 } 382 }
383 } 383 }
384 384
385 // Some non-pure instructions may be treated as pure. HLocalGet depends on
386 // assignments, but we can ignore the initializing assignment since it will by
387 // construction always precede a use.
388 bool isEffectivelyPure(HInstruction instruction) {
389 if (instruction is HLocalGet) return !isAssignedLocal(instruction.local);
390 return instruction.isPure();
391 }
392
393 bool isAssignedLocal(HLocalValue local) {
394 // [HLocalValue]s have an initializing assignment which is guaranteed to
395 // precede the use, except for [HParameterValue]s which are 'assigned' at
396 // entry.
397 int initializingAssignmentCount = (local is HParameterValue) ? 0 : 1;
398 return local.usedBy
399 .where((user) => user is HLocalSet)
400 .skip(initializingAssignmentCount)
401 .isNotEmpty;
402 }
403
404 void visitInstruction(HInstruction instruction) { 385 void visitInstruction(HInstruction instruction) {
405 // A code motion invariant instruction is dealt before visiting it. 386 // A code motion invariant instruction is dealt before visiting it.
406 assert(!instruction.isCodeMotionInvariant()); 387 assert(!instruction.isCodeMotionInvariant());
407 analyzeInputs(instruction, 0); 388 analyzeInputs(instruction, 0);
408 } 389 }
409 390
410 void visitInvokeSuper(HInvokeSuper instruction) { 391 void visitInvokeSuper(HInvokeSuper instruction) {
411 Element superMethod = instruction.element; 392 Element superMethod = instruction.element;
412 Selector selector = instruction.selector; 393 Selector selector = instruction.selector;
413 // If aliased super members cannot be used, we will generate code like 394 // If aliased super members cannot be used, we will generate code like
(...skipping 79 matching lines...) Expand 10 before | Expand all | Expand 10 after
493 // included in the expression. 474 // included in the expression.
494 475
495 // The expectedInputs list holds non-trivial instructions that may 476 // The expectedInputs list holds non-trivial instructions that may
496 // be generated at their use site, if they occur in the correct order. 477 // be generated at their use site, if they occur in the correct order.
497 if (expectedInputs == null) expectedInputs = new List<HInstruction>(); 478 if (expectedInputs == null) expectedInputs = new List<HInstruction>();
498 if (pureInputs == null) pureInputs = new Set<HInstruction>(); 479 if (pureInputs == null) pureInputs = new Set<HInstruction>();
499 480
500 // Pop instructions from expectedInputs until instruction is found. 481 // Pop instructions from expectedInputs until instruction is found.
501 // Return true if it is found, or false if not. 482 // Return true if it is found, or false if not.
502 bool findInInputsAndPopNonMatching(HInstruction instruction) { 483 bool findInInputsAndPopNonMatching(HInstruction instruction) {
503 assert(!isEffectivelyPure(instruction)); 484 assert(!instruction.isPure());
504 while (!expectedInputs.isEmpty) { 485 while (!expectedInputs.isEmpty) {
505 HInstruction nextInput = expectedInputs.removeLast(); 486 HInstruction nextInput = expectedInputs.removeLast();
506 assert(!generateAtUseSite.contains(nextInput)); 487 assert(!generateAtUseSite.contains(nextInput));
507 assert(nextInput.usedBy.length == 1); 488 assert(nextInput.usedBy.length == 1);
508 if (identical(nextInput, instruction)) { 489 if (identical(nextInput, instruction)) {
509 return true; 490 return true;
510 } 491 }
511 } 492 }
512 return false; 493 return false;
513 } 494 }
514 495
515 block.last.accept(this); 496 block.last.accept(this);
516 for (HInstruction instruction = block.last.previous; 497 for (HInstruction instruction = block.last.previous;
517 instruction != null; 498 instruction != null;
518 instruction = instruction.previous) { 499 instruction = instruction.previous) {
519 if (generateAtUseSite.contains(instruction)) { 500 if (generateAtUseSite.contains(instruction)) {
520 continue; 501 continue;
521 } 502 }
522 if (instruction.isCodeMotionInvariant()) { 503 if (instruction.isCodeMotionInvariant()) {
523 markAsGenerateAtUseSite(instruction); 504 markAsGenerateAtUseSite(instruction);
524 continue; 505 continue;
525 } 506 }
526 if (isEffectivelyPure(instruction)) { 507 if (instruction.isPure()) {
527 if (pureInputs.contains(instruction)) { 508 if (pureInputs.contains(instruction)) {
528 tryGenerateAtUseSite(instruction); 509 tryGenerateAtUseSite(instruction);
529 } else { 510 } else {
530 // If the input is not in the [pureInputs] set, it has not 511 // If the input is not in the [pureInputs] set, it has not
531 // been visited or should not be generated at use-site. The most 512 // been visited or should not be generated at use-site. The most
532 // likely reason for the latter, is that the instruction is used 513 // likely reason for the latter, is that the instruction is used
533 // in more than one location. 514 // in more than one location.
534 // We must either clear the expectedInputs, or move the pure 515 // We must either clear the expectedInputs, or move the pure
535 // instruction's inputs in front of the existing ones. 516 // instruction's inputs in front of the existing ones.
536 // Example: 517 // Example:
(...skipping 25 matching lines...) Expand all
562 // use(t3); 543 // use(t3);
563 // 544 //
564 // If we clear the expected-inputs we can't generate-at-use any of 545 // If we clear the expected-inputs we can't generate-at-use any of
565 // the instructions. 546 // the instructions.
566 // 547 //
567 // The optimal solution is to move the inputs of 'pure' in 548 // The optimal solution is to move the inputs of 'pure' in
568 // front of the expectedInputs list. This makes sense, since we 549 // front of the expectedInputs list. This makes sense, since we
569 // push expected-inputs from left-to right, and the `pure` function 550 // push expected-inputs from left-to right, and the `pure` function
570 // invocation is "more left" (i.e. before) the first argument of `f`. 551 // invocation is "more left" (i.e. before) the first argument of `f`.
571 // With that approach we end up with: 552 // With that approach we end up with:
572 // t3 = pure(foo()); 553 // t3 = pure(foo();
573 // f(bar(), t3); 554 // f(bar(), t3);
574 // use(t3); 555 // use(t3);
575 // 556 //
576 int oldLength = expectedInputs.length; 557 int oldLength = expectedInputs.length;
577 instruction.accept(this); 558 instruction.accept(this);
578 if (oldLength != 0 && oldLength != expectedInputs.length) { 559 if (oldLength != 0 && oldLength != expectedInputs.length) {
579 // Move the pure instruction's inputs to the front. 560 // Move the pure instruction's inputs to the front.
580 List<HInstruction> newInputs = expectedInputs.sublist(oldLength); 561 List<HInstruction> newInputs = expectedInputs.sublist(oldLength);
581 int newCount = newInputs.length; 562 int newCount = newInputs.length;
582 expectedInputs.setRange( 563 expectedInputs.setRange(
(...skipping 194 matching lines...) Expand 10 before | Expand all | Expand 10 after
777 } 758 }
778 759
779 // If [thenInput] is defined in the first predecessor, then it is only used 760 // If [thenInput] is defined in the first predecessor, then it is only used
780 // by [phi] and can be generated at use site. 761 // by [phi] and can be generated at use site.
781 if (identical(thenInput.block, end.predecessors[0])) { 762 if (identical(thenInput.block, end.predecessors[0])) {
782 assert(thenInput.usedBy.length == 1); 763 assert(thenInput.usedBy.length == 1);
783 markAsGenerateAtUseSite(thenInput); 764 markAsGenerateAtUseSite(thenInput);
784 } 765 }
785 } 766 }
786 } 767 }
OLDNEW
« no previous file with comments | « no previous file | pkg/compiler/lib/src/ssa/optimize.dart » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698