[llvm-commits] [llvm] r50987 - /llvm/trunk/lib/CodeGen/MachineLICM.cpp

[Bill Wendling]

Yeah. I realized my error afterwards. Reverted.

-bw

On Mon, May 12, 2008 at 12:44 PM, Evan Cheng <evan.cheng at apple.com> wrote:
> This is wrong. We definitely *want* to hoist instructions that are
> rematerializable. These instructions can be remat at the uses in case
> LICM increases register pressure and spilling ensues.
>
> Evan
>
> On May 12, 2008, at 12:38 PM, Bill Wendling wrote:
>
>> Author: void
>> Date: Mon May 12 14:38:32 2008
>> New Revision: 50987
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=50987&view=rev
>> Log:
>> One real change - don't hoist something that's trivially
>> rematerializable. It's
>> possible for it to produce worse code than before.
>>
>> The rest of this patch is code cleanup.
>>
>> Modified:
>>    llvm/trunk/lib/CodeGen/MachineLICM.cpp
>>
>> Modified: llvm/trunk/lib/CodeGen/MachineLICM.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineLICM.cpp?rev=50987&r1=50986&r2=50987&view=diff
>>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> ======================================================================
>> --- llvm/trunk/lib/CodeGen/MachineLICM.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/MachineLICM.cpp Mon May 12 14:38:32 2008
>> @@ -34,16 +34,16 @@
>>   class VISIBILITY_HIDDEN MachineLICM : public MachineFunctionPass {
>>     const TargetMachine   *TM;
>>     const TargetInstrInfo *TII;
>> -    MachineFunction       *CurMF; // Current MachineFunction
>> +    MachineFunction       *CurMF;  // Current MachineFunction
>>
>>     // Various analyses that we use...
>> -    MachineLoopInfo      *LI;   // Current MachineLoopInfo
>> -    MachineDominatorTree *DT;   // Machine dominator tree for the
>> current Loop
>> +    MachineLoopInfo      *LI;      // Current MachineLoopInfo
>> +    MachineDominatorTree *DT;      // Machine dominator tree for
>> the cur loop
>>     MachineRegisterInfo  *RegInfo; // Machine register information
>>
>>     // State that is updated as we process loops
>> -    bool         Changed;       // True if a loop is changed.
>> -    MachineLoop *CurLoop;       // The current loop we are working
>> on.
>> +    bool         Changed;          // True if a loop is changed.
>> +    MachineLoop *CurLoop;          // The current loop we are
>> working on.
>>   public:
>>     static char ID; // Pass identification, replacement for typeid
>>     MachineLICM() : MachineFunctionPass((intptr_t)&ID) {}
>> @@ -233,15 +233,14 @@
>>     return false;
>>
>>   if (TID.mayLoad()) {
>> -    // Okay, this instruction does a load.  As a refinement, allow
>> the target
>> -    // to decide whether the loaded value is actually a constant.
>> If so, we
>> -    // can actually use it as a load.
>> -    if (!TII->isInvariantLoad(&I)) {
>> +    // Okay, this instruction does a load. As a refinement, we
>> allow the target
>> +    // to decide whether the loaded value is actually a constant.
>> If so, we can
>> +    // actually use it as a load.
>> +    if (!TII->isInvariantLoad(&I))
>>       // FIXME: we should be able to sink loads with no other side
>> effects if
>>       // there is nothing that can change memory from here until the
>> end of
>> -      // block.  This is a trivial form of alias analysis.
>> +      // block. This is a trivial form of alias analysis.
>>       return false;
>> -    }
>>   }
>>
>>   DEBUG({
>> @@ -263,12 +262,9 @@
>>              *ImpDefs; ++ImpDefs)
>>           DOUT << "      -> " << TRI->getName(*ImpDefs) << "\n";
>>       }
>> -
>> -        //if (TII->hasUnmodelledSideEffects(&I))
>> -        //DOUT << "  * Instruction has side effects.\n";
>>     });
>>
>> -  // The instruction is loop invariant if all of its operands are
>> loop-invariant
>> +  // The instruction is loop invariant if all of its operands are.
>>   for (unsigned i = 0, e = I.getNumOperands(); i != e; ++i) {
>>     const MachineOperand &MO = I.getOperand(i);
>>
>> @@ -282,7 +278,8 @@
>>     if (TargetRegisterInfo::isPhysicalRegister(Reg))
>>       return false;
>>
>> -    assert(RegInfo->getVRegDef(Reg)&&"Machine instr not mapped for
>> this vreg?");
>> +    assert(RegInfo->getVRegDef(Reg) &&
>> +           "Machine instr not mapped for this vreg?!");
>>
>>     // If the loop contains the definition of an operand, then the
>> instruction
>>     // isn't loop invariant.
>> @@ -294,12 +291,16 @@
>>   return true;
>> }
>>
>> -/// Hoist - When an instruction is found to only use loop invariant
>> operands
>> -/// that is safe to hoist, this instruction is called to do the
>> dirty work.
>> +/// Hoist - When an instruction is found to use only loop invariant
>> operands
>> +/// that are safe to hoist, this instruction is called to do the
>> dirty work.
>> ///
>> void MachineLICM::Hoist(MachineInstr &MI) {
>>   if (!IsLoopInvariantInst(MI)) return;
>>
>> +  // Hoisting things that are trivially rematerializable may result
>> in worse
>> +  // code than before.
>> +  if (TII->isTriviallyReMaterializable(&MI)) return;
>> +
>>   std::vector<MachineBasicBlock*> Preds;
>>
>>   // Non-back-edge predecessors.
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>