Title: [204857] trunk/Source/_javascript_Core
- Revision
- 204857
- Author
- [email protected]
- Date
- 2016-08-23 13:43:56 -0700 (Tue, 23 Aug 2016)
Log Message
Spilling of constant tmps should make it easier for the spill code optimizer to rematerialize the constant
https://bugs.webkit.org/show_bug.cgi?id=160150
Reviewed by Benjamin Poulain.
When we spill in-place for admitsStack()==true, we prevent rematerialization if that
argument doesn't also admit immediates (which it almost certainly won't do). So, we
prevent remat.
This fixes the issue by avoiding in-place spilling for warm uses of constants. I don't
know if this helps performance, but I do know that it make the codegen for
bigswitch-indirect-symbol look a lot better. Prior to this change, the prolog would have
a constant materialization for each symbol that function used, and then it would spill
that constant. This removes all of that yucky code.
This also changes how IRC detects constant Tmps. Previously we would say that a Tmp is a
constant if the number of const defs was equal to the number of defs. But it's possible
for each of the const defs to produce a different value. This is unlikely considering
how B3->Air lowering works and how our SSA works - each def would have its own register.
But, regardless, this picks a more precise way of detecting constants: the number of
const defs must be 1 and the number of defs must be 1.
* b3/air/AirIteratedRegisterCoalescing.cpp:
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (204856 => 204857)
--- trunk/Source/_javascript_Core/ChangeLog 2016-08-23 20:29:35 UTC (rev 204856)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-08-23 20:43:56 UTC (rev 204857)
@@ -1,3 +1,29 @@
+2016-07-24 Filip Pizlo <[email protected]>
+
+ Spilling of constant tmps should make it easier for the spill code optimizer to rematerialize the constant
+ https://bugs.webkit.org/show_bug.cgi?id=160150
+
+ Reviewed by Benjamin Poulain.
+
+ When we spill in-place for admitsStack()==true, we prevent rematerialization if that
+ argument doesn't also admit immediates (which it almost certainly won't do). So, we
+ prevent remat.
+
+ This fixes the issue by avoiding in-place spilling for warm uses of constants. I don't
+ know if this helps performance, but I do know that it make the codegen for
+ bigswitch-indirect-symbol look a lot better. Prior to this change, the prolog would have
+ a constant materialization for each symbol that function used, and then it would spill
+ that constant. This removes all of that yucky code.
+
+ This also changes how IRC detects constant Tmps. Previously we would say that a Tmp is a
+ constant if the number of const defs was equal to the number of defs. But it's possible
+ for each of the const defs to produce a different value. This is unlikely considering
+ how B3->Air lowering works and how our SSA works - each def would have its own register.
+ But, regardless, this picks a more precise way of detecting constants: the number of
+ const defs must be 1 and the number of defs must be 1.
+
+ * b3/air/AirIteratedRegisterCoalescing.cpp:
+
2016-08-23 Filip Pizlo <[email protected]>
Unreviewed, fix CLoop build.
Modified: trunk/Source/_javascript_Core/b3/air/AirIteratedRegisterCoalescing.cpp (204856 => 204857)
--- trunk/Source/_javascript_Core/b3/air/AirIteratedRegisterCoalescing.cpp 2016-08-23 20:29:35 UTC (rev 204856)
+++ trunk/Source/_javascript_Core/b3/air/AirIteratedRegisterCoalescing.cpp 2016-08-23 20:43:56 UTC (rev 204857)
@@ -1121,7 +1121,7 @@
// If it's a constant, then it's not as bad to spill. We can rematerialize it in many
// cases.
- if (counts->numConstDefs == counts->numDefs)
+ if (counts->numConstDefs == 1 && counts->numDefs == 1)
uses /= 2;
return degree / uses;
@@ -1462,25 +1462,41 @@
// Try to replace the register use by memory use when possible.
inst.forEachArg(
[&] (Arg& arg, Arg::Role role, Arg::Type argType, Arg::Width width) {
- if (arg.isTmp() && argType == type && !arg.isReg()) {
- auto stackSlotEntry = stackSlots.find(arg.tmp());
- if (stackSlotEntry != stackSlots.end()
- && inst.admitsStack(arg)) {
-
- Arg::Width spillWidth = m_tmpWidth.requiredWidth(arg.tmp());
- if (Arg::isAnyDef(role) && width < spillWidth)
- return;
- ASSERT(inst.opcode == Move || !(Arg::isAnyUse(role) && width > spillWidth));
-
- if (spillWidth != Arg::Width32)
- canUseMove32IfDidSpill = false;
-
- stackSlotEntry->value->ensureSize(
- canUseMove32IfDidSpill ? 4 : Arg::bytes(width));
- arg = Arg::stack(stackSlotEntry->value);
- didSpill = true;
- }
+ if (!arg.isTmp())
+ return;
+ if (argType != type)
+ return;
+ if (arg.isReg())
+ return;
+
+ auto stackSlotEntry = stackSlots.find(arg.tmp());
+ if (stackSlotEntry == stackSlots.end())
+ return;
+ if (!inst.admitsStack(arg))
+ return;
+
+ // If the Tmp holds a constant then we want to rematerialize its
+ // value rather than loading it from the stack. In order for that
+ // optimization to kick in, we need to avoid placing the Tmp's stack
+ // address into the instruction.
+ if (!Arg::isColdUse(role)) {
+ const UseCounts<Tmp>::Counts* counts = m_useCounts[arg.tmp()];
+ if (counts && counts->numConstDefs == 1 && counts->numDefs == 1)
+ return;
}
+
+ Arg::Width spillWidth = m_tmpWidth.requiredWidth(arg.tmp());
+ if (Arg::isAnyDef(role) && width < spillWidth)
+ return;
+ ASSERT(inst.opcode == Move || !(Arg::isAnyUse(role) && width > spillWidth));
+
+ if (spillWidth != Arg::Width32)
+ canUseMove32IfDidSpill = false;
+
+ stackSlotEntry->value->ensureSize(
+ canUseMove32IfDidSpill ? 4 : Arg::bytes(width));
+ arg = Arg::stack(stackSlotEntry->value);
+ didSpill = true;
});
if (didSpill && canUseMove32IfDidSpill)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
