llvm dev - Feb 2013 - [LLVMdev] eliminateCallFramePseudoInstr belongs in TargetRegisterInfo or TargetFrameLowering

Hi all,

I ran into this while trying to figure out why the X86
getSUBriOpcode/getADDriOpcode functions are duplicated, appearing once
in X86RegisterInfo.cpp and once in X86FrameLowering.cpp,

The method TargetRegisterInfo::eliminateCallFramePseudoInstr doesn't
appear to really belong in this interface. It adds instructions into
the MachineFunction given to it, which isn't what TargetRegisterInfo
is supposed to do.

ISTM that eliminateCallFramePseudoInstr belongs in
TargerFrameLowering, since it's being used during prolog/epilog
insertion. Moving it there would avoid the code duplication and
possibly other layering problems.

What do you think

Eli

P.S. I'm asking in llvmdev before proposing an actual patch because
this change will affect all targets.

P.S.2 Alas, TargetFrameLowering is not documented in
http://llvm.org/docs/CodeGenerator.html

> ISTM that eliminateCallFramePseudoInstr belongs in
> TargerFrameLowering, since it's being used during prolog/epilog
> insertion. Moving it there would avoid the code duplication and
> possibly other layering problems.
> What do you think
Go ahead and move. It's s historical artifact why it is inside TRI.

-- 
With best regards, Anton Korobeynikov
Faculty of Mathematics and Mechanics, Saint Petersburg State University

Done in r175788, thanks.


Note: this touches the target interfaces, so out-of-tree targets may
be affected.


On Tue, Feb 19, 2013 at 12:55 PM, Anton Korobeynikov
<anton at korobeynikov.info> wrote:
>> ISTM that eliminateCallFramePseudoInstr belongs in
>> TargerFrameLowering, since it's being used during prolog/epilog
>> insertion. Moving it there would avoid the code duplication and
>> possibly other layering problems.
>> What do you think
> Go ahead and move. It's s historical artifact why it is inside TRI.
>
> --
> With best regards, Anton Korobeynikov
> Faculty of Mathematics and Mechanics, Saint Petersburg State University