Commit 8e833623 authored by mtrofin's avatar mtrofin Committed by Commit bot

Revert of MIPS: Fix bad RegisterConfiguration usage in InstructionSequence...

Revert of MIPS: Fix bad RegisterConfiguration usage in InstructionSequence unit tests. (patchset #3 id:40001 of https://codereview.chromium.org/2433093002/ )

Reason for revert:
This change rendered InstructionSequenceTest::SetNumRegs ineffectual, thus
loosening the tests that were using that API to ensure correct register
allocation under intentionally constrained setups.

For the problem stated in this CL, a solution needs to continue supporting the
intentionally set-up test configuration.

Original issue's description:
> MIPS: Fix bad RegisterConfiguration usage in InstructionSequence unit tests.
>
> Test InstructionSequenceTest has been initialized with a testing RegisterConfiguration
> instance defined in instruction-sequence-unittest.h, whereas class ExplicitOperand which
> is being tested used RegisterConfiguration from instruction.cc. In case these two
> instances are different, the tests would fail. The issue is fixed by using the same
> instance of RegisterConfiguration both for test code and code under test.
>
> Additionally, the tests in register-allocator-unittest.cc use hardcoded values
> for register and begin failing is the hardcoded register is not available for
> allocation. Fix by forcing the use of allocatable registers only.
>
> TEST=unittests.MoveOptimizerTest.RemovesRedundantExplicit,unittests.RegisterAllocatorTest.SpillPhi
> BUG=
>
> Committed: https://crrev.com/0cf56232209d4c9c669b8426680de18806f6c29a
> Cr-Commit-Position: refs/heads/master@{#40862}

TBR=dcarney@chromium.org,bmeurer@chromium.org,mstarzinger@chromium.org,vogelheim@chromium.org,titzer@chromium.org,ivica.bogosavljevic@imgtec.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=

Review-Url: https://codereview.chromium.org/2587593002
Cr-Commit-Position: refs/heads/master@{#41777}
parent 61833f5b
......@@ -12,8 +12,7 @@ namespace v8 {
namespace internal {
namespace compiler {
const RegisterConfiguration* (*GetRegConfig)() =
RegisterConfiguration::Turbofan;
const auto GetRegConfig = RegisterConfiguration::Turbofan;
FlagsCondition CommuteFlagsCondition(FlagsCondition condition) {
switch (condition) {
......@@ -986,11 +985,6 @@ void InstructionSequence::PrintBlock(int block_id) const {
PrintBlock(GetRegConfig(), block_id);
}
const RegisterConfiguration*
InstructionSequence::GetRegisterConfigurationForTesting() {
return GetRegConfig();
}
FrameStateDescriptor::FrameStateDescriptor(
Zone* zone, FrameStateType type, BailoutId bailout_id,
OutputFrameStateCombine state_combine, size_t parameters_count,
......
......@@ -1570,8 +1570,6 @@ class V8_EXPORT_PRIVATE InstructionSequence final
void ValidateDeferredBlockEntryPaths() const;
void ValidateSSA() const;
const RegisterConfiguration* GetRegisterConfigurationForTesting();
private:
friend V8_EXPORT_PRIVATE std::ostream& operator<<(
std::ostream& os, const PrintableInstructionSequence& code);
......
......@@ -19,6 +19,11 @@ static const char*
static char register_names_[10 * (RegisterConfiguration::kMaxGeneralRegisters +
RegisterConfiguration::kMaxFPRegisters)];
namespace {
static int allocatable_codes[InstructionSequenceTest::kDefaultNRegs] = {
0, 1, 2, 3, 4, 5, 6, 7};
}
static void InitializeRegisterNames() {
char* loc = register_names_;
for (int i = 0; i < RegisterConfiguration::kMaxGeneralRegisters; ++i) {
......@@ -80,8 +85,19 @@ int InstructionSequenceTest::GetAllocatableCode(int index,
}
}
const RegisterConfiguration* InstructionSequenceTest::config() {
return sequence()->GetRegisterConfigurationForTesting();
RegisterConfiguration* InstructionSequenceTest::config() {
if (!config_) {
config_.reset(new RegisterConfiguration(
num_general_registers_, num_double_registers_, num_general_registers_,
num_double_registers_, allocatable_codes, allocatable_codes,
kSimpleFPAliasing ? RegisterConfiguration::OVERLAP
: RegisterConfiguration::COMBINE,
general_register_names_,
double_register_names_, // float register names
double_register_names_,
double_register_names_)); // SIMD 128 register names
}
return config_.get();
}
......
......@@ -158,7 +158,7 @@ class InstructionSequenceTest : public TestWithIsolateAndZone {
void SetNumRegs(int num_general_registers, int num_double_registers);
int GetNumRegs(MachineRepresentation rep);
int GetAllocatableCode(int index, MachineRepresentation rep = kNoRep);
const RegisterConfiguration* config();
RegisterConfiguration* config();
InstructionSequence* sequence();
void StartLoop(int loop_blocks);
......
......@@ -321,11 +321,11 @@ TEST_F(RegisterAllocatorTest, SpillPhi) {
EndBlock(Branch(Imm(), 1, 2));
StartBlock();
auto left = Define(Reg(GetAllocatableCode(0)));
auto left = Define(Reg(0));
EndBlock(Jump(2));
StartBlock();
auto right = Define(Reg(GetAllocatableCode(0)));
auto right = Define(Reg(0));
EndBlock();
StartBlock();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment