[llvm-commits] CVS: llvm/lib/Target/X86/X86InstrFPStack.td X86InstrInfo.cpp X86InstrInfo.h X86InstrInfo.td X86InstrMMX.td X86InstrSSE.td

[Chris Lattner]

> Replace M_REMATERIALIZIBLE and the newly-added  
> isOtherReMaterializableLoad
> with a general target hook to identify rematerializable  
> instructions. Some
> instructions are only rematerializable with specific operands, such  
> as loads
...

>  // Floating point constant loads.
> -let isReMaterializable = 1 in {
>  def FpLD0 : FpI<(ops RFP:$dst), ZeroArgFP,
>                  [(set RFP:$dst, fp64imm0)]>;
>  def FpLD1 : FpI<(ops RFP:$dst), ZeroArgFP,
>                  [(set RFP:$dst, fp64imm1)]>;
> -}

> +bool X86InstrInfo::isTriviallyReMaterializable(MachineInstr *MI)  
> const {
>    switch (MI->getOpcode()) {
>    default: break;
> +  case X86::FpLD0:
> +  case X86::FpLD1:
> +  case X86::MOV8ri:
> +  case X86::MOV16ri:
> +  case X86::MOV32ri:
> +  case X86::MMX_V_SET0:
> +  case X86::MMX_V_SETALLONES:
> +  case X86::V_SET0:
> +  case X86::V_SETALLONES:
> +    // These instructions are always trivially rematerializable.
> +    return true;
>    case X86::MOV8rm:
>    case X86::MOV16rm:
>    case X86::MOV16_rm:
> @@ -128,6 +139,7 @@
>    case X86::MOVAPDrm:
>    case X86::MMX_MOVD64rm:
>    case X86::MMX_MOVQ64rm:
> +    // Loads from constant pools are trivially rematerializable.
>      return MI->getOperand(1).isRegister() && MI->getOperand 
> (2).isImmediate() &&
>             MI->getOperand(3).isRegister() && MI->getOperand 
> (4).isConstantPoolIndex() &&
>             MI->getOperand(1).getReg() == 0 &&


Hi Dan,

I'm sorry to be such a pain, but this seems like a step backward.   
We've gone from having an explicit flag in the .td files to having  
another magic table in the .cpp file (somewhat amusing because  
'duplicable' just made the opposite transition).

How about this proposal (Obviously feel free to pick better names for  
these things):

1. Reintroduce the 'isremat-able' flag, set it to true for all the  
instructions that are *potentially* rematerializable.
2. Add a virtual target hook that can override the flag:  
"TII::isReallyRematerializable(Machineinstr*)".
3. Introduce a new non-virtual method:
   bool TII::isRematerializable(Machineinstr *MI) {
     return MI->flags->isrematable && isReallyRematerializable(MI);
   }

This achieves two things:

1. Just looking at the .td file, you can tell which instructions are  
candidates for remat.
2. The isRematerializable predicate is faster for instructions that  
are not remat-able.
3. The isReallyRematerializable only needs to be implemented by  
targets with instructions that are remat-able only in some cases  
(like the x86 instructions).

To me, #1 is the killer feature.  In general, I'd like to move away  
from having tables (either explicit, like the one in  
X86RegisterInfo::foldMemoryOperand, or just big switch stmts) to  
having properties on .td file entries.  That makes it much more clear  
what is going on when inspecting the .td files.

I'm sorry I didn't look at your patch when you asked for comments,  
but does this proposal sound sane?

-Chris